Skip to content

Commit 3036401

Browse files
committed
Clarify best practices and update anti-patterns list
1 parent 8bbba57 commit 3036401

File tree

3 files changed

+4
-72
lines changed

3 files changed

+4
-72
lines changed

lib/elixir/pages/anti-patterns/code-anti-patterns.md

Lines changed: 2 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ You could refactor the code above like this:
3434
@five_min_in_seconds 60 * 5
3535

3636
defp unix_five_min_from_now do
37-
unix_now = DateTime.to_unix(DateTime.utc_now(), :second)
37+
now = DateTime.utc_now()
38+
unix_now = DateTime.to_unix(now, :second)
3839
unix_now + @five_min_in_seconds
3940
end
4041
```
@@ -45,75 +46,6 @@ We removed the unnecessary comments. We also added a `@five_min_in_seconds` modu
4546

4647
Elixir makes a clear distinction between **documentation** and code comments. The language has built-in first-class support for documentation through `@doc`, `@moduledoc`, and more. See the ["Writing documentation"](../getting-started/writing-documentation.md) guide for more information.
4748

48-
## Complex branching
49-
50-
#### Problem
51-
52-
When a function assumes the responsibility of handling multiple errors alone, it can increase its cyclomatic complexity (metric of control-flow) and become incomprehensible. This situation can configure a specific instance of "Long function", a traditional anti-pattern, but has implications of its own. Under these circumstances, this function could get very confusing, difficult to maintain and test, and therefore bug-proneness.
53-
54-
#### Example
55-
56-
An example of this anti-pattern is when a function uses the `case` control-flow structure or other similar constructs (for example, `cond` or `receive`) to handle variations of a return type. This practice can make the function more complex, long, and difficult to understand, as shown next.
57-
58-
```elixir
59-
def get_customer(customer_id) do
60-
case SomeHTTPClient.get("/customers/#{customer_id}") do
61-
{:ok, %{status: 200, body: body}} ->
62-
case Jason.decode(body) do
63-
{:ok, decoded} ->
64-
%{
65-
"first_name" => first_name,
66-
"last_name" => last_name,
67-
"company" => company
68-
} = decoded
69-
70-
customer =
71-
%Customer{
72-
id: customer_id,
73-
name: "#{first_name} #{last_name}",
74-
company: company
75-
}
76-
77-
{:ok, customer}
78-
79-
{:error, _} ->
80-
{:error, "invalid response body"}
81-
end
82-
83-
{:error, %{status: status, body: body}} ->
84-
case Jason.decode(body) do
85-
%{"error" => message} when is_binary(message) ->
86-
{:error, message}
87-
88-
%{} ->
89-
{:error, "invalid response with status #{status}"}
90-
end
91-
end
92-
end
93-
```
94-
95-
The code above is complex because the `case` clauses are long and often have their own branching logic in them. With the clauses spread out, it is hard to understand what each clause does individually and it is hard to see all of the different scenarios your code pattern matches on.
96-
97-
#### Refactoring
98-
99-
As shown below, in this situation, instead of concentrating all error handling within the same function, creating complex branches, it is better to delegate each branch to a different private function. In this way, the code will be cleaner and more readable.
100-
101-
```elixir
102-
def get_customer(customer_id) do
103-
case SomeHTTPClient.get("/customers/#{customer_id}") do
104-
{:ok, %{status: 200, body: body}} ->
105-
http_customer_to_struct(customer_id, body)
106-
107-
{:error, %{status: status, body: body}} ->
108-
http_error(status, body)
109-
end
110-
end
111-
```
112-
113-
Both `http_customer_to_struct(customer_id, body)` and `http_error(status, body)` above contain the previous branches refactored into private functions.
114-
115-
It is worth noting that this refactoring is trivial to perform in Elixir because clauses cannot define variables or otherwise affect their parent scope. Therefore, extracting any clause or branch to a private function is a matter of gathering all variables used in that branch and passing them as arguments to the new function.
116-
11749
## Complex `else` clauses in `with`
11850

11951
#### Problem

lib/elixir/pages/anti-patterns/what-anti-patterns.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,4 @@ Each anti-pattern is documented using the following structure:
3838

3939
The initial catalog of anti-patterns was proposed by Lucas Vegi and Marco Tulio Valente, from [ASERG/DCC/UFMG](http://aserg.labsoft.dcc.ufmg.br/). For more info, see [Understanding Code Smells in Elixir Functional Language](https://github.com/lucasvegi/Elixir-Code-Smells/blob/main/etc/2023-emse-code-smells-elixir.pdf) and [the associated code repository](https://github.com/lucasvegi/Elixir-Code-Smells).
4040

41-
Additionally, the Security Working Group of the [Erlang Ecosystem Foundation](https://erlef.github.io/security-wg/) publishes [documents with security resources and best-practices of both Erland and Elixir, including detailed guides for web applications](https://erlef.github.io/security-wg/).
41+
Additionally, the Security Working Group of the [Erlang Ecosystem Foundation](https://erlef.github.io/security-wg/) publishes [documents with security resources and best-practices of both Erlang and Elixir, including detailed guides for web applications](https://erlef.github.io/security-wg/).

lib/elixir/pages/getting-started/alias-require-and-import.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ end
112112

113113
In the example above, the imported `List.duplicate/2` is only visible within that specific function. `duplicate/2` won't be available in any other function in that module (or any other module for that matter).
114114

115-
Note that `import`s are generally discouraged in the language. When working on your own code, prefer `alias` to `import`.
115+
While `import`s can be a useful for frameworks and libraries to build abstractions, developers should generally prefer `alias` to `import` on their own codebases, as aliases make the origin of the function being invoked clearer.
116116

117117
## use
118118

0 commit comments

Comments
 (0)