Matter AI | Code Reviewer Documentation

Erlang Anti-Patterns Overview

Using Processes for Everything

%% Anti-pattern: Using processes unnecessarily
start() ->
    spawn(fun() -> calculator_loop() end).

calculator_loop() ->
    receive
        {add, A, B, Caller} ->
            Caller ! {result, A + B},
            calculator_loop();
        {subtract, A, B, Caller} ->
            Caller ! {result, A - B},
            calculator_loop()
    end.

%% Usage
use_calculator() ->
    Calc = start(),
    Calc ! {add, 2, 3, self()},
    receive
        {result, Result} -> Result
    end.

%% Better approach: Use regular functions for simple operations
add(A, B) -> A + B.
subtract(A, B) -> A - B.

%% Usage
use_calculator() ->
    add(2, 3).

Don’t use processes for simple operations that don’t need concurrency, state isolation, or fault tolerance. Processes have overhead and should be used when their benefits are needed.

Not Using OTP Behaviors

%% Anti-pattern: Reinventing OTP patterns
-module(user_store).
-export([start/0, store/3, retrieve/2]).

start() ->
    spawn(fun() -> loop(#{}) end).

store(Pid, Key, Value) ->
    Pid ! {store, Key, Value}.

retrieve(Pid, Key) ->
    Pid ! {retrieve, Key, self()},
    receive
        {value, Value} -> Value
    after 1000 ->
        {error, timeout}
    end.

loop(State) ->
    receive
        {store, Key, Value} ->
            loop(maps:put(Key, Value, State));
        {retrieve, Key, Caller} ->
            Caller ! {value, maps:get(Key, State, undefined)},
            loop(State)
    end.

%% Better approach: Use gen_server
-module(user_store).
-behaviour(gen_server).

%% API
-export([start_link/0, store/2, retrieve/1]).

%% gen_server callbacks
-export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]).

start_link() ->
    gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).

store(Key, Value) ->
    gen_server:cast(?MODULE, {store, Key, Value}).

retrieve(Key) ->
    gen_server:call(?MODULE, {retrieve, Key}).

%% Callbacks
init([]) ->
    {ok, #{}}.

handle_call({retrieve, Key}, _From, State) ->
    {reply, maps:get(Key, State, undefined), State}.

handle_cast({store, Key, Value}, State) ->
    {noreply, maps:put(Key, Value, State)}.

handle_info(_Info, State) ->
    {noreply, State}.

terminate(_Reason, _State) ->
    ok.

code_change(_OldVsn, State, _Extra) ->
    {ok, State}.

Use OTP behaviors like gen_server, supervisor, and application instead of reinventing them. They provide battle-tested solutions for common patterns.

Using List Concatenation in Loops

%% Anti-pattern: Inefficient list concatenation
process_items(Items) ->
    process_items(Items, []).

process_items([], Acc) ->
    Acc;
process_items([Item | Rest], Acc) ->
    Processed = process_item(Item),
    process_items(Rest, Acc ++ [Processed]). %% Inefficient: O(n) operation

%% Better approach: Use list prepending and reverse
process_items(Items) ->
    process_items(Items, []).

process_items([], Acc) ->
    lists:reverse(Acc); %% Reverse once at the end
process_items([Item | Rest], Acc) ->
    Processed = process_item(Item),
    process_items(Rest, [Processed | Acc]). %% Efficient: O(1) operation

Avoid using ++ for list concatenation in loops. Instead, use list prepending ([Head | Tail]) and reverse the result at the end if order matters.

Not Using Pattern Matching Effectively

%% Anti-pattern: Not using pattern matching
process_response(Response) ->
    case Response of
        {ok, Body} ->
            %% Process data
            {ok, process_body(Body)};
        {error, Reason} ->
            %% Handle error
            {error, Reason}
    end.

%% Better approach: Use pattern matching in function heads
process_response({ok, Body}) ->
    %% Process data
    {ok, process_body(Body)};
process_response({error, Reason}) ->
    %% Handle error
    {error, Reason}.

Leverage Erlang’s pattern matching for cleaner, more declarative code. Use it in function heads and case statements.

Using String Operations Inefficiently

%% Anti-pattern: Inefficient string operations
join_strings(Strings) ->
    join_strings(Strings, "").

join_strings([], Acc) ->
    Acc;
join_strings([String | Rest], Acc) ->
    join_strings(Rest, Acc ++ String). %% Inefficient for strings

%% Better approach: Use binaries for strings
join_binaries(Binaries) ->
    join_binaries(Binaries, <<>>).

join_binaries([], Acc) ->
    Acc;
join_binaries([Binary | Rest], Acc) ->
    join_binaries(Rest, <<Acc/binary, Binary/binary>>).

Use binaries instead of lists for string operations. Binaries are more efficient for string manipulation in Erlang.

Not Using Records or Maps for Structured Data

%% Anti-pattern: Using tuples for structured data
create_user(Name, Email, Age) ->
    {user, Name, Email, Age}.

get_user_name({user, Name, _Email, _Age}) ->
    Name.

%% Better approach: Use records
-record(user, {name, email, age}).

create_user(Name, Email, Age) ->
    #user{name = Name, email = Email, age = Age}.

get_user_name(#user{name = Name}) ->
    Name.

%% Or even better in modern Erlang: Use maps
create_user(Name, Email, Age) ->
    #{type => user, name => Name, email => Email, age => Age}.

get_user_name(#{type := user, name := Name}) ->
    Name.

Use records or maps for structured data instead of raw tuples. They provide named fields and better pattern matching.

Not Using Proper Error Handling

%% Anti-pattern: Ignoring errors
read_file(Filename) ->
    case file:read_file(Filename) of
        {ok, Content} ->
            Content;
        {error, _Reason} ->
            <<>> %% Silent failure with empty binary
    end.

%% Better approach: Proper error handling
read_file(Filename) ->
    case file:read_file(Filename) of
        {ok, Content} ->
            {ok, Content};
        {error, Reason} ->
            {error, {file_read_error, Filename, Reason}}
    end.

Use proper error handling with tagged tuples ({ok, Result} and {error, Reason}) and propagate errors up the call stack.

Not Using try/catch Appropriately

%% Anti-pattern: Excessive use of try/catch
divide(A, B) ->
    try
        A / B
    catch
        error:badarith ->
            0
    end.

%% Better approach: Use pattern matching and guards
divide(A, B) when B /= 0 ->
    {ok, A / B};
divide(_A, 0) ->
    {error, division_by_zero}.

Avoid using try/catch for control flow. In Erlang, it’s more idiomatic to use pattern matching, guard clauses, and tagged tuples for error handling.

Not Using Proper Supervision Trees

%% Anti-pattern: Manual process management
start_system() ->
    DatabasePid = database:start_link(),
    CachePid = cache:start_link(),
    ApiPid = api:start_link(DatabasePid, CachePid),
    {ok, ApiPid}.
%% What if one of these crashes?

%% Better approach: Use supervision trees
-module(my_app_sup).
-behaviour(supervisor).

-export([start_link/0, init/1]).

start_link() ->
    supervisor:start_link({local, ?MODULE}, ?MODULE, []).

init([]) ->
    SupFlags = #{strategy => one_for_one, intensity => 10, period => 60},
    ChildSpecs = [
        #{id => database, start => {database, start_link, []}, restart => permanent, shutdown => 5000, type => worker},
        #{id => cache, start => {cache, start_link, []}, restart => permanent, shutdown => 5000, type => worker},
        #{id => api, start => {api, start_link, []}, restart => permanent, shutdown => 5000, type => worker}
    ],
    {ok, {SupFlags, ChildSpecs}}.

Use proper supervision trees to manage process lifecycles and handle failures. This is a core strength of the Erlang VM.

Using send/receive Without Timeouts

%% Anti-pattern: receive without timeout
get_data(Pid) ->
    Pid ! {get_data, self()},
    receive
        {data, Data} -> Data
    end. %% Could block forever

%% Better approach: Always use timeouts with receive
get_data(Pid) ->
    Pid ! {get_data, self()},
    receive
        {data, Data} -> {ok, Data}
    after 5000 ->
        {error, timeout}
    end.

Always use timeouts with receive to prevent processes from blocking indefinitely if the expected message never arrives.

Not Using ETS Tables Appropriately

%% Anti-pattern: Using processes for simple shared state
-module(counter).
-export([start/0, increment/1, get/1]).

start() ->
    spawn(fun() -> loop(0) end).

increment(Pid) ->
    Pid ! increment,
    ok.

get(Pid) ->
    Pid ! {get, self()},
    receive
        {count, Count} -> Count
    after 1000 ->
        {error, timeout}
    end.

loop(Count) ->
    receive
        increment ->
            loop(Count + 1);
        {get, Pid} ->
            Pid ! {count, Count},
            loop(Count)
    end.

%% Better approach: Use ETS for shared state
-module(counter).
-export([start/0, increment/0, get/0]).

-define(TABLE, counter_table).

start() ->
    ets:new(?TABLE, [named_table, public]),
    ets:insert(?TABLE, {count, 0}),
    ok.

increment() ->
    ets:update_counter(?TABLE, count, 1),
    ok.

get() ->
    [{count, Count}] = ets:lookup(?TABLE, count),
    Count.

Use ETS (Erlang Term Storage) tables for shared state when appropriate, especially for read-heavy scenarios or when low-latency access is required.

Not Using Proper Module Structure

%% Anti-pattern: Poor module structure
-module(user_system).
-export([create_user/1, update_user/2, delete_user/1, get_user/1, list_users/0,
         authenticate_user/2, generate_password_reset_token/1]).

%% Hundreds of functions in one module
create_user(Params) -> % ...
update_user(Id, Params) -> % ...
delete_user(Id) -> % ...
get_user(Id) -> % ...
list_users() -> % ...
authenticate_user(Email, Password) -> % ...
generate_password_reset_token(Email) -> % ...
%% And many more...

%% Better approach: Proper module organization
%% users.erl
-module(users).
-export([create/1, update/2, delete/1, get/1, list/0]).

create(Params) -> % ...
update(Id, Params) -> % ...
delete(Id) -> % ...
get(Id) -> % ...
list() -> % ...

%% users_auth.erl
-module(users_auth).
-export([authenticate/2, generate_password_reset_token/1]).

authenticate(Email, Password) -> % ...
generate_password_reset_token(Email) -> % ...

Organize your code into cohesive modules with clear responsibilities. Follow the principle of single responsibility and create a logical hierarchy of modules.

Not Using Proper Documentation

%% Anti-pattern: Poor or no documentation
-module(user_api).
-export([create/1]).

create(Params) ->
    %% Implementation...
    {ok, User}.

%% Better approach: Proper documentation
-module(user_api).
-export([create/1]).

%% @doc Creates a new user with the given parameters.
%%
%% Params is a map containing user attributes:
%% <ul>
%%   <li>`name': The user's full name (required)</li>
%%   <li>`email': The user's email address (required)</li>
%%   <li>`age': The user's age (optional)</li>
%% </ul>
%%
%% Examples:
%% ```
%% {ok, User} = user_api:create(#{name => "John Doe", email => "john@example.com"}).
%% {error, email_required} = user_api:create(#{name => "John Doe"}).
%% '''
%%
%% @returns `{ok, User}' if the user was created successfully, or
%%          `{error, Reason}' if there was an error.
-spec create(map()) -> {ok, map()} | {error, atom()}.
create(Params) ->
    %% Implementation...
    {ok, User}.

Write comprehensive documentation for your modules and functions using EDoc comments. Include examples, parameter descriptions, and return value information.

Not Using Dialyzer and Type Specs

%% Anti-pattern: No type specs
-module(calculator).
-export([add/2, divide/2]).

add(A, B) ->
    A + B.

divide(A, B) ->
    A / B.

%% Better approach: Use type specs
-module(calculator).
-export([add/2, divide/2]).

-spec add(number(), number()) -> number().
add(A, B) ->
    A + B.

-spec divide(number(), number()) -> float() | {error, division_by_zero}.
divide(A, B) when B /= 0 ->
    A / B;
divide(_A, 0) ->
    {error, division_by_zero}.

Use Dialyzer and type specifications to catch type errors at compile time and improve code documentation.

Not Using Proper Application Structure

%% Anti-pattern: Ad-hoc application structure
%% Just a collection of modules without proper OTP structure

%% Better approach: Proper OTP application structure
%% my_app.app.src
{application, my_app, [
    {description, "My Application"},
    {vsn, "0.1.0"},
    {registered, []},
    {mod, {my_app_app, []}},
    {applications, [kernel, stdlib]},
    {env, []},
    {modules, []}
]}.

%% my_app_app.erl
-module(my_app_app).
-behaviour(application).
-export([start/2, stop/1]).

start(_StartType, _StartArgs) ->
    my_app_sup:start_link().

stop(_State) ->
    ok.

%% my_app_sup.erl
-module(my_app_sup).
-behaviour(supervisor).
-export([start_link/0, init/1]).

start_link() ->
    supervisor:start_link({local, ?MODULE}, ?MODULE, []).

init([]) ->
    SupFlags = #{strategy => one_for_one, intensity => 10, period => 60},
    ChildSpecs = [
        %% Child specifications
    ],
    {ok, {SupFlags, ChildSpecs}}.

Follow the OTP application structure with proper application, supervisor, and worker modules.

Not Using Proper Process Naming

%% Anti-pattern: Not registering important processes
start() ->
    Pid = spawn(fun() -> loop([]) end),
    Pid. %% Caller needs to keep track of the Pid

%% Usage
use_service() ->
    Pid = start(),
    Pid ! {get_data, self()},
    receive
        {data, Data} -> Data
    end.

%% Better approach: Register important processes
start() ->
    Pid = spawn(fun() -> loop([]) end),
    register(my_service, Pid),
    {ok, Pid}.

%% Usage
use_service() ->
    my_service ! {get_data, self()},
    receive
        {data, Data} -> Data
    end.

Register important long-lived processes with names for easier access. This is especially important for system processes that need to be accessed from multiple places.

Not Using Proper Message Patterns

%% Anti-pattern: Ambiguous message patterns
loop(State) ->
    receive
        {get, Key} ->
            Value = maps:get(Key, State, undefined),
            %% Who sent this message? No way to reply
            loop(State);
        {set, Key, Value} ->
            loop(maps:put(Key, Value, State))
    end.

%% Better approach: Include sender in messages
loop(State) ->
    receive
        {get, Key, From} ->
            Value = maps:get(Key, State, undefined),
            From ! {response, Value},
            loop(State);
        {set, Key, Value, From} ->
            From ! ok,
            loop(maps:put(Key, Value, State))
    end.

Include the sender’s PID in messages that require a response. This allows the receiving process to reply to the correct sender.

Using list:foreach Instead of list Comprehensions

%% Anti-pattern: Using lists:foreach for transformations
process_items(Items) ->
    Results = [],
    lists:foreach(fun(Item) ->
        Result = process_item(Item),
        Results = [Result | Results] %% This doesn't work as expected!
    end, Items),
    lists:reverse(Results).

%% Better approach: Use list comprehensions or lists:map
process_items(Items) ->
    [process_item(Item) || Item <- Items].

%% Or
process_items(Items) ->
    lists:map(fun process_item/1, Items).

Use list comprehensions or lists:map for transforming lists. lists:foreach is only for side effects and doesn’t return a useful value.

Not Using Proper Testing

%% Anti-pattern: Manual testing
test() ->
    5 = add(2, 3),
    io:format("Test passed!~n").

%% Better approach: Use EUnit
-module(calculator_tests).
-include_lib("eunit/include/eunit.hrl").

add_test() ->
    ?assertEqual(5, calculator:add(2, 3)),
    ?assertEqual(0, calculator:add(-2, 2)),
    ?assertEqual(-5, calculator:add(-2, -3)).

divide_test() ->
    ?assertEqual(2.5, calculator:divide(5, 2)),
    ?assertEqual({error, division_by_zero}, calculator:divide(5, 0)).

Use proper testing frameworks like EUnit or Common Test instead of writing manual test code.

Introduction

Getting Started

Product

Patterns

Integrations

Enterprise

Erlang