ic-facet/elixir-antipatterns

Elixir Anti-Patterns

Critical anti-patterns that compromise robustness and maintainability in Elixir/Phoenix applications.

Complement with: mix format and Credo for style enforcement
Extended reference: See EXTENDED.md for 40+ patterns and deep-dive examples

---

When to Use

Topics: Error handling (3 patterns) • Architecture (2 patterns) • Performance (2 patterns) • Testing (1 pattern)

Load this skill when:

• Writing Elixir modules and functions
• Working with Phoenix Framework (Controllers, LiveView)
• Building Ecto schemas and database queries
• Implementing BEAM concurrency (Task, GenServer)
• Handling errors with tagged tuples
• Writing tests with ExUnit

---

Critical Patterns

Quick reference to the 8 core patterns this skill enforces:

1. Tagged Tuples: Return {:ok, value} | {:error, reason} instead of nil or exceptions
2. Explicit @spec: Document error cases in function signatures
3. Context Separation: Business logic in contexts, not LiveView
4. Preload Associations: Use Repo.preload/2 to avoid N+1 queries
5. with Arrow Binding: Use <- for all failable operations in with
6. Database Indexes: Index frequently queried columns
7. Test Assertions: Every test must assert expected behavior
8. Cohesive Functions: Group with chains >4 steps into functions

See ## Anti-Patterns section below for detailed ❌ BAD / ✅ CORRECT code examples.

---

Code Examples

Example 1: Error Handling with Tagged Tuples

# ✅ CORRECT - Errors as values, explicit in @spec
defmodule UserService do
  @spec fetch_user(String.t()) :: {:ok, User.t()} | {:error, :not_found}
  def fetch_user(id) do
    case Repo.get(User, id) do
      nil -> {:error, :not_found}
      user -> {:ok, user}
    end
  end
end

# ❌ BAD - Exceptions for business errors
def fetch_user(id) do
  Repo.get(User, id) || raise "User not found"
end

Example 2: Phoenix LiveView with Context Separation

Architecture Layers:
  User Request → LiveView (UI only) → Context (business logic) → Schema/Repo (data)
               ↓                    ↓                           ↓
           handle_event()     Accounts.create_user()      Repo.insert()

# ✅ CORRECT - Thin LiveView, logic in context
defmodule MyAppWeb.UserLive.Index do
  use MyAppWeb, :live_view
  
  def handle_event("create", params, socket) do
    case Accounts.create_user(params) do
      {:ok, user} -> {:noreply, redirect(socket, to: ~p"/users/#{user}")}
      {:error, changeset} -> {:noreply, assign(socket, changeset: changeset)}
    end
  end
end

# ❌ BAD - Business logic in LiveView
def handle_event("create", %{"user" => params}, socket) do
  if String.length(params["name"]) < 3 do
    {:noreply, put_flash(socket, :error, "Too short")}
  else
    case Repo.insert(User.changeset(%User{}, params)) do
      {:ok, user} -> send_email(user); redirect(socket)
    end
  end
end

Example 3: Ecto N+1 Query Optimization

# ✅ CORRECT - Preload associations (2 queries total)
users = User |> Repo.all() |> Repo.preload(:posts)
Enum.map(users, fn user -> process(user, user.posts) end)

# Note: For complex filtering (e.g., WHERE posts.status = 'published'),
# use join + preload in the query itself. See EXTENDED.md for advanced patterns.

# ❌ BAD - Query in loop (101 queries for 100 users)
users = Repo.all(User)
Enum.map(users, fn user ->
  posts = Repo.all(from p in Post, where: p.user_id == ^user.id)
  {user, posts}
end)

---

Anti-Patterns

Error Management

Don't: Use raise for Business Errors

# ❌ BAD
def fetch_user(id) do
  Repo.get(User, id) || raise "User not found"
end

# ✅ CORRECT
@spec fetch_user(String.t()) :: {:ok, User.t()} | {:error, :not_found}
def fetch_user(id) do
  case Repo.get(User, id) do
    nil -> {:error, :not_found}
    user -> {:ok, user}
  end
end

Why: @spec documents errors, pattern matching forces explicit handling.

---

Don't: Return nil for Errors

# ❌ BAD - No context on failure
def find_user(email), do: Repo.get_by(User, email: email)

# ✅ CORRECT - Explicit error reason
@spec find_user(String.t()) :: {:ok, User.t()} | {:error, :not_found}
def find_user(email) do
  case Repo.get_by(User, email: email) do
    nil -> {:error, :not_found}
    user -> {:ok, user}
  end
end

---

Don't: Use = Inside with for Failable Operations

# ❌ BAD - Validate errors silenced
with {:ok, user} <- fetch_user(id),
     validated = validate(user),  # ← Doesn't check for {:error, _}
     {:ok, saved} <- save(validated) do
  {:ok, saved}
end

# ✅ CORRECT - All operations use <-
with {:ok, user} <- fetch_user(id),
     {:ok, validated} <- validate(user),
     {:ok, saved} <- save(validated) do
  {:ok, saved}
end

---

Architecture & Boundaries

Don't: Put Business Logic in LiveView

# ❌ BAD - Validation in view
def handle_event("create", %{"user" => params}, socket) do
  if String.length(params["name"]) < 3 do
    {:noreply, put_flash(socket, :error, "Too short")}
  else
    case Repo.insert(User.changeset(%User{}, params)) do
      {:ok, user} -> redirect(socket)
    end
  end
end

# ✅ CORRECT - Delegate to context
def handle_event("create", params, socket) do
  case Accounts.create_user(params) do
    {:ok, user} -> {:noreply, redirect(socket, to: ~p"/users/#{user}")}
    {:error, changeset} -> {:noreply, assign(socket, changeset: changeset)}
  end
end

Why: Contexts testable without Phoenix, logic reusable.

---

Don't: Chain More Than 4 Steps in with

# ❌ BAD - Too many responsibilities
with {:ok, a} <- step1(),
     {:ok, b} <- step2(a),
     {:ok, c} <- step3(b),
     {:ok, d} <- step4(c),
     {:ok, e} <- step5(d) do
  {:ok, e}
end

# ✅ CORRECT - Group into cohesive functions
with {:ok, validated} <- validate_and_fetch(id),
     {:ok, processed} <- process_business_rules(validated),
     {:ok, result} <- persist_and_notify(processed) do
  {:ok, result}
end

---

Data & Performance

Don't: Query Inside Loops (N+1)

# ❌ BAD - 101 queries for 100 users
users = Repo.all(User)
Enum.map(users, fn user ->
  posts = Repo.all(from p in Post, where: p.user_id == ^user.id)
end)

# ✅ CORRECT - 2 queries total
User |> Repo.all() |> Repo.preload(:posts)

Impact: 100 users with N+1 = 10 seconds vs 5ms with preload.

---

Don't: Query Without Indexes

# ❌ BAD - No index on frequently queried column
# Migration:
create table(:users) do
  add :email, :string
end

# ✅ CORRECT - Add index
create table(:users) do
  add :email, :string
end
create unique_index(:users, [:email])

Why: Full table scan on 1M+ rows vs instant index lookup.

---

Testing

Don't: Write Tests Without Assertions

# ❌ BAD - What's being tested?
test "creates user" do
  UserService.create_user(%{name: "Juan"})
end

# ✅ CORRECT - Assert expected behavior
test "creates user successfully" do
  assert {:ok, user} = UserService.create_user(%{name: "Juan"})
  assert user.name == "Juan"
end

---

Quick Reference

| Situation | Anti-Pattern | Correct Pattern | |-----------|--------------|-----------------| | Error handling | raise "Not found" | {:error, :not_found} | | Missing data | Return nil | {:error, :not_found} | | Business logic | In LiveView | In context modules | | Associations | Enum.map + Repo.get | Repo.preload | | with chains | validated = fn() | {:ok, validated} <- fn() | | Frequent queries | No index | create index(:table, [:column]) | | Testing | No assertions | assert expected behavior | | Complex logic | 6+ step with | Group into 3 functions |

---

Resources

• Elixir Style Guide
• Phoenix Contexts
• Ecto Query Performance
• ExUnit Best Practices
• Extended patterns: See EXTENDED.md for 40+ anti-patterns