Help us improve
Share bugs, ideas, or general feedback.
From elixir
Identifies and refactors Elixir anti-patterns like excessive comments, complex `with` else clauses, and complex variable extractions. Use for reviewing code smells, refactoring, and improving quality.
npx claudepluginhub vinnie357/claude-skills --plugin elixirHow this skill is triggered — by the user, by Claude, or both
Slash command
/elixir:anti-patternsThe summary Claude sees in its skill listing — used to decide when to auto-load this skill
You are an expert at identifying Elixir anti-patterns and suggesting idiomatic refactorings. Use this knowledge to analyze code, suggest improvements, and help developers write better Elixir.
Detects and refactors Elixir anti-patterns including comment overuse, complex with/else clauses, and messy function heads.
Enforces Elixir best practices like pattern matching over if/else, pipe operator for chaining, with for sequential fallible ops, @impl true, and let-it-crash when editing .ex/.exs files.
Guides idiomatic Elixir style: avoids bang functions in business logic, uses pattern matching for errors, designs Ecto schemas for validation.
Share bugs, ideas, or general feedback.
You are an expert at identifying Elixir anti-patterns and suggesting idiomatic refactorings. Use this knowledge to analyze code, suggest improvements, and help developers write better Elixir.
Problem: Excessive or self-explanatory comments reduce readability rather than enhance it.
Detection:
Refactoring:
@doc and @moduledoc for documentationExample:
# Bad
def calculate() do
# Get current time
now = DateTime.utc_now()
# Add 5 minutes
DateTime.add(now, 5 * 60, :second)
end
# Good
@minutes_to_add 5
def timestamp_five_minutes_from_now do
now = DateTime.utc_now()
DateTime.add(now, @minutes_to_add * 60, :second)
end
else Clauses in withProblem: Flattening all error handling into a single complex else block obscures which clause produced which error.
Detection:
else blocks with many pattern match clauseselseRefactoring:
with focus on success pathsExample:
# Bad
def read_config(path) do
with {:ok, content} <- File.read(path),
{:ok, decoded} <- Jason.decode(content) do
{:ok, decoded}
else
{:error, :enoent} -> {:error, :file_not_found}
{:error, %Jason.DecodeError{}} -> {:error, :invalid_json}
{:error, reason} -> {:error, reason}
end
end
# Good
def read_config(path) do
with {:ok, content} <- read_file(path),
{:ok, config} <- parse_json(content) do
{:ok, config}
end
end
defp read_file(path) do
case File.read(path) do
{:ok, content} -> {:ok, content}
{:error, :enoent} -> {:error, :file_not_found}
error -> error
end
end
defp parse_json(content) do
case Jason.decode(content) do
{:ok, data} -> {:ok, data}
{:error, _} -> {:error, :invalid_json}
end
end
Problem: Extracting values across multiple clauses and arguments makes it unclear which variables serve pattern/guard purposes versus function body usage.
Detection:
Refactoring:
%User{age: age} = userExample:
# Bad
def process(%User{age: age, name: name, email: email} = user) when age >= 18 do
# Only using name and email in body, not age
send_email(email, "Hello #{name}")
end
# Good
def process(%User{age: age} = user) when age >= 18 do
send_email(user.email, "Hello #{user.name}")
end
Problem: Atoms aren't garbage-collected and are limited to ~1 million. Uncontrolled dynamic atom creation poses memory and security risks.
Detection:
String.to_atom/1 with untrusted inputRefactoring:
String.to_existing_atom/1 with pre-defined atomsExample:
# Bad - Security risk!
def set_role(user, role_string) do
%{user | role: String.to_atom(role_string)}
end
# Good
def set_role(user, role) when role in [:admin, :editor, :viewer] do
%{user | role: role}
end
# Or with pattern matching
def set_role(user, "admin"), do: %{user | role: :admin}
def set_role(user, "editor"), do: %{user | role: :editor}
def set_role(user, "viewer"), do: %{user | role: :viewer}
def set_role(_user, invalid), do: {:error, "Invalid role: #{invalid}"}
Problem: Functions with excessive parameters become confusing and error-prone to use.
Detection:
Refactoring:
Example:
# Bad
def create_loan(user_id, user_name, user_email, book_id, book_title, book_isbn) do
# ...
end
# Good
def create_loan(user, book) do
# ...
end
# Or with keyword list for options
def create_loan(user, book, opts \\ []) do
duration = Keyword.get(opts, :duration, 14)
renewable = Keyword.get(opts, :renewable, true)
# ...
end
Problem: Defining modules outside your library's namespace risks conflicts since the Erlang VM loads only one module instance per name.
Detection:
Plug.* when you're not Plug)Refactoring:
Example:
# Bad - Library named :plug_auth
defmodule Plug.Auth do
# This conflicts with the actual Plug library!
end
# Good
defmodule PlugAuth do
# ...
end
defmodule PlugAuth.Session do
# ...
end
Problem: Using dynamic access (map[:key]) for required keys masks missing data, allowing nil to propagate instead of failing fast.
Detection:
map[:key] for required/expected keysRefactoring:
map.key) for required keysExample:
# Bad
def distance(point) do
x = point[:x] # Returns nil if :x is missing!
y = point[:y]
:math.sqrt(x * x + y * y) # Crashes on nil, but unclear why
end
# Good
def distance(%{x: x, y: y}) do
:math.sqrt(x * x + y * y) # Clear error if keys missing
end
# Or with structs
defmodule Point do
defstruct [:x, :y]
end
def distance(%Point{x: x, y: y}) do
:math.sqrt(x * x + y * y)
end
Problem: Writing defensive code that returns incorrect values instead of using pattern matching to assert expected structures causes silent failures.
Detection:
Refactoring:
Example:
# Bad
def parse_query_param(param) do
case String.split(param, "=") do
[key, value] -> {key, value}
_ -> {"", ""} # Silent failure!
end
end
# Good
def parse_query_param(param) do
[key, value] = String.split(param, "=")
{key, value}
end
# Crashes with clear error if format is wrong - this is good!
Problem: Using truthiness operators (&&, ||, !) when all operands are boolean is unnecessarily generic and unclear.
Detection:
&&, ||, ! with boolean expressionsis_binary(x) && is_integer(y)Refactoring:
and, or, not for boolean-only operations&&, ||, ! for truthy/falsy logicExample:
# Bad
def valid_user?(name, age) do
is_binary(name) && is_integer(age) && age >= 18
end
# Good
def valid_user?(name, age) do
is_binary(name) and is_integer(age) and age >= 18
end
# Truthy operators are OK for nil/value checks
def get_name(user) do
user[:name] || "Anonymous"
end
Problem: Structs with 32+ fields switch from Erlang's efficient flat-map representation to hash maps, increasing memory usage.
Detection:
Refactoring:
Example:
# Bad
defmodule User do
defstruct [
:id, :email, :name, :age, :address, :city, :state, :zip,
:phone, :mobile, :fax, :company, :title, :department,
:created_at, :updated_at, :last_login, :login_count,
:preference1, :preference2, :preference3, :preference4,
# ... 15 more fields
]
end
# Good
defmodule User do
defstruct [
:id,
:email,
:name,
:profile, # Nested struct
:preferences, # Nested struct
:metadata # Nested struct
]
end
defmodule User.Profile do
defstruct [:age, :phone, :mobile, :address, :city, :state, :zip]
end
defmodule User.Preferences do
defstruct [:theme, :notifications, :language]
end
Problem: Functions with options that drastically change their return type make it unclear what the function actually returns.
Detection:
Refactoring:
Example:
# Bad
def parse(string, opts \\ []) do
case Integer.parse(string) do
{int, rest} ->
if opts[:discard_rest], do: int, else: {int, rest}
:error ->
:error
end
end
# Good
def parse(string) do
case Integer.parse(string) do
{int, rest} -> {int, rest}
:error -> :error
end
end
def parse_discard_rest(string) do
case Integer.parse(string) do
{int, _rest} -> int
:error -> :error
end
end
Problem: Using multiple booleans with overlapping states instead of atoms or composite types to represent domain concepts.
Detection:
Refactoring:
Example:
# Bad
def create_user(name, email, admin: false, editor: false, viewer: true) do
# What if admin: true, editor: true?
end
# Good
def create_user(name, email, role: :viewer) do
# Clear: role can be :admin, :editor, or :viewer
end
Problem: Using try/rescue for expected errors instead of pattern matching with case statements and tuple returns.
Detection:
try/rescue blocks for normal operation errors! functions and rescuingRefactoring:
{:ok, value} or {:error, reason}Example:
# Bad
def read_config(path) do
try do
content = File.read!(path)
Jason.decode!(content)
rescue
e -> {:error, e}
end
end
# Good
def read_config(path) do
with {:ok, content} <- File.read(path),
{:ok, config} <- Jason.decode(content) do
{:ok, config}
end
end
Problem: Excessively using basic types (strings, integers) instead of creating composite types to represent structured domain concepts.
Detection:
Refactoring:
Example:
# Bad
def create_address(street, city, state, zip, country) do
# All strings, no validation
"#{street}, #{city}, #{state} #{zip}, #{country}"
end
# Good
defmodule Address do
defstruct [:street, :city, :state, :zip, :country]
def new(attrs) do
struct!(__MODULE__, attrs)
end
def format(%__MODULE__{} = address) do
"#{address.street}, #{address.city}, #{address.state} #{address.zip}, #{address.country}"
end
end
Problem: Grouping completely unrelated business logic into one multi-clause function.
Detection:
Refactoring:
Example:
# Bad
def update(%Product{} = product) do
# Product-specific logic
end
def update(%Animal{} = animal) do
# Completely different animal logic
end
# Good
def update_product(%Product{} = product) do
# Product-specific logic
end
def update_animal(%Animal{} = animal) do
# Animal-specific logic
end
# Or use a protocol
defprotocol Updatable do
def update(item)
end
defimpl Updatable, for: Product do
def update(product), do: # ...
end
defimpl Updatable, for: Animal do
def update(animal), do: # ...
end
Problem: Libraries relying on global application environment configuration prevent multiple dependent applications from configuring the library differently.
Detection:
Application.get_env/2 or Application.fetch_env!/2 in library codeRefactoring:
Example:
# Bad - Library code
def split(string) do
parts = Application.fetch_env!(:dash_splitter, :parts)
String.split(string, "-", parts: parts)
end
# Good
def split(string, opts \\ []) do
parts = Keyword.get(opts, :parts, 2)
String.split(string, "-", parts: parts)
end
When reviewing or writing Elixir code: