Credo's Elixir Style Guide

Prelude

There are two reasons for this document to exist:

It is my personal style guide and consists both of the way I write Elixir today, but more importantly of things I've seen in the wild and adapted because they make Elixir more readable for open source Alchemists everywhere.
Secondly, it is the basis for Credo and reflects the principles promoted by its code analysis.

Like all of my work, this style guide stands on the shoulders of giants: It is influenced by the Ruby style guides by bbatsov and GitHub as well as more public attempts at Elixir Style Guides.

Philosophy

Contrary to other guides I've seen, this one is not very dogmatic. The overall principles are

be consistent in your choices (i.e. apply the same rules everywhere),
care about the readability of your code (e.g. when in doubt, spread text vertically rather than horizontally),
and care about easier maintenance (avoid confusing names, etc.).

This is especially important because we are such a young community. All the code we put out there is worth its weight in gold if it is easy to comprehend and invites people to learn and contribute.

Contribute

If you want to add to this document, please submit a pull request or open an issue to discuss specific points.

The Actual Guide

Code Readability

Use tabs consistently (2 spaces soft-tabs are preferred). ^[link]
Use line-endings consistently (Unix-style line endings are preferred, but we should not exclude our brothers and sisters riding the Redmond dragon). ^[link]
Don't leave trailing white-space at the end of a line. ^[link]
End each file with a newline (some editors don't do this by default). ^[link]
Use spaces around operators and after commas. ^[link]

Don't use spaces after (, [, and { or before }, ], and ). This is the preferred way, although other styles are possible, as long as they are applied consistently. ^[link]

# preferred way
Helper.format({1, true, 2}, :my_atom)

# also okay - carefully choose a style and use it consistently
Helper.format( { 1, true, 2 }, :my_atom )

Keep lines fewer than 80 characters whenever possible, although this is not a strict rule. ^[link]

Don't use ; to separate statements and expressions. ^[link]

# preferred way
IO.puts "Waiting for:"
IO.inspect object

# NOT okay
IO.puts "Waiting for:"; IO.inspect object

Don't put a space after ! to negate an expression. ^[link]

# preferred way
denied = !allowed?

# NOT okay
denied = ! allowed?

Group function definitions. Keep the same function with different signatures together without separating blank lines. In all other cases, use blank lines to separate different functions/parts of your module (to maximize readability through "vertical white-space"). ^[link]

defp find_properties(source_file, config) do
  {property_for(source_file, config), source_file}
end

defp property_for(source_file, _config) do
  Enum.map(lines, &tabs_or_spaces/1)
end

defp tabs_or_spaces({_, "\t" <> line}), do: :tabs
defp tabs_or_spaces({_, "  " <> line}), do: :spaces
defp tabs_or_spaces({_, line}), do: nil

Generally use vertical-space to improve readability of sections of your code. ^[link]

# it is preferred to employ a mixture of parentheses, descriptive variable
# names and vertical white space to improve readability

def run(%SourceFile{} = source_file, params \\ []) do
  source_file
  |> Helper.find_unused_calls(params, [:String], nil)
  |> Enum.reduce([], &add_to_issues/2)
end

defp add_to_issues(invalid_call, issues) do
  {trigger, meta, _} = invalid_call

  issues ++ [issue(meta[:line], trigger, source_file)]
end

# this function does the same as above, but is less comprehensible

def run(%SourceFile{} = source_file, params \\ []) do
  Helper.find_unused_calls(source_file, params, [:String], nil)
  |> Enum.reduce [], fn {_, meta, _} = invalid_call, issues ->
    trigger = invalid_call |> Macro.to_string |> String.split("(") |> List.first
    issues ++ [issue(meta[:line], trigger, source_file)]
  end
end

It is preferred to start pipe chains with a "pure" value rather than a function call. ^[link]

# preferred way - this is very readable due to the clear flow of data
username
|> String.strip
|> String.downcase

# also okay - but often slightly less readable
String.strip(username)
|> String.downcase

When assigning to a multi-line call, begin a new line after the =. Indent the assigned value's calculation by one level. This is the preferred way. ^[link]

# preferred way
result =
  lines
  |> Enum.map(&tabs_or_spaces/1)
  |> Enum.uniq

# also okay - align the first assignment and subsequent lines
result = lines
         |> Enum.map(&tabs_or_spaces/1)
         |> Enum.uniq

Add underscores to large numbers for better readability. ^[link]

# preferred way - very easy to read
num = 10_000_000

# NOT okay - how many zeros are there?
num = 10000000

Use def, defp, and defmacro with parentheses when the function takes parameters. Omit the parentheses when the function doesn't accept any parameters. This is the preferred way. ^[link]

# preferred way - omit parentheses for functions without parameters
def time do
  # ...
end

# use parentheses if parameters are present
def convert(x, y) do
  # ...
end

Most of the time when calling functions that take parameters, it is preferred to use parentheses. ^[link]

# preferred way - the more boring forms are preferred since it's easier to see what goes where
Enum.reduce(1..100, 0, &(&1 + &2))

Enum.reduce(1..100, 0, fn(x, acc) ->
  x + acc
end)

# also okay - carefully choose a style and use it consistently
Enum.reduce 1..100, 0, & &1 + &2

Enum.reduce 1..100, 0, fn x, acc ->
  x + acc
end

For macros we see the contrary behaviour. The preferred way is to not use parentheses. ^[link]

# preferred way
defmodule MyApp.Service.TwitterAPI do
  use MyApp.Service, social: true

  alias MyApp.Service.Helper, as: H
end

Conclusively, never use parentheses around the condition of if or unless (since they are macros as well). ^[link]

# preferred way
if valid?(username) do
  # ...
end

# NOT okay
if( valid?(username) ) do
  # ...
end

Naming

Use CamelCase for module names. It is preferred to keep acronyms like HTTP, XML uppercase. ^[link]

# preferred way
defmodule MyApp.HTTPService do
end

# also okay - carefully choose a style and use it consistently
defmodule MyApp.HttpService do
end

Use snake_case for module attribute, function, macro and variable names. ^[link]

# preferred way
defmodule MyApp.HTTPService do
  @some_setting :my_value

  def my_function(param_value) do
    variable_value1 = "test"
  end
end

# NOT okay
defmodule MyApp.HTTPService do
  @someSetting :my_value

  def myFunction(paramValue) do
    variableValue1 = "test"
  end
end

Exception names should have a common prefix or suffix. While this can be anything you like, esp. for small libraries, a common choice seems to have all of them end in Error. ^[link]

# preferred way - common suffix Error
defmodule BadHTTPHeaderError do
  defexception [:message]
end

defmodule HTTPRequestError do
  defexception [:message]
end

# also okay - consistent prefix Invalid
defmodule InvalidHTTPHeader do
  defexception [:message]
end

defmodule InvalidUserRequest do
  defexception [:message]
end

# bad - there is no common naming scheme for exceptions
defmodule InvalidHeader do
  defexception [:message]
end

defmodule RequestFailed do
  defexception [:message]
end

Predicate functions/macros should return a boolean value. ^[link]

For functions, they should end in a question mark.

# preferred way
def valid?(username) do
  # ...
end

# NOT okay
def is_valid?(username) do
  # ...
end

For guard-safe macros they should have the prefix is_ and not end in a question mark.

# preferred way
defmacro is_valid(username) do
  # ...
end

# NOT okay
defmacro valid?(username) do
  # ...
end

Sigils

Use sigils where it makes sense, but don't use them in a dogmatic way. ^[link]

Example: Don't automatically use ~S just because there is one " in your string, but start using it when you would have to escape a lot of double-quotes.

# preferred way - use normal quotes even if one has to be escaped
legend = "single quote ('), double quote (\")"

# use sigils when you would have to escape several quotes otherwise
html = ~S(<a href="http://elixir-lang.org" target="_blank" rel="external">Homepage</a>)

# also okay, but not preferred - important: choose a common sigil and stick with it
# avoid using ~S{} in one place while using ~S(), ~S[] and ~S<> in others
legend = ~S{single quote ('), double quote (")}
html = "<a href=\"http://elixir-lang.org\" target=\"_blank\" rel=\"external\">Homepage</a>"

Regular Expressions

Use ~r// as your "go-to sigil" when it comes to Regexes as they are the easiest to read for people new to Elixir. That said, feel free to use other ~r sigils when you have several slashes in your expression. ^[link]

# preferred way - use slashes because they are familiar regex delimiters
regex = ~r/\d+/

# use sigils when you would have to escape several quotes otherwise
regex = ~r{http://elixir-lang.org/getting-started/mix-otp/(.+).html}

Be careful with ^ and $ as they match start/end of line, not string endings. If you want to match the whole string use: \A and \z. ^[link]

Documentation

First, try to see documentation in a positive light. It is not a chore that you put off for as long as possible (forever). At some point in your project, starting to document parts of your project is an opportunity to communicate important things with future-maintainers, potential users of your API and even your future self. ^[link]
When that point in time comes, every module and function should either be documented or marked via @moduledoc false/@doc false to indicate that there is no intent to document the object in question. ^[link]
Use ExDoc for this, it's great. ^[link]

As for style, put an empty line after @moduledoc. Don't put an empty line between @doc and its function/macro definition. ^[link]

defmodule MyApp.HTTPService do
  @moduledoc false

  @doc "Sends a POST request to the given `url`."
  def post(url) do
    # ...
  end
end

Although Elixir favors @moduledoc and @doc as first-class citizens, don't be afraid to communicate via normal code comments as well. But remember: don't use this to explain bad code! ^[link]

# preferred way - provide useful additional information
defmodule Credo.Issue do
  defstruct category:     nil,
            message:      nil,
            filename:     nil,
            line_no:      nil,
            column:       nil,
            trigger:      nil,  # optional: the call that triggered the issue
            metadata:     [],   # optional: filled in by the failing check
end

# NOT okay - "explaining" confusing code and ambiguous names
defmodule AbstractCredoIssueInterfaceFactory do
  def build(input, params \\ []) do
    Helper.find_values(input, params) # input is either a username or pid
    |> Enum.reduce %{}, fn {_, meta, _} = value, list ->
      if valid?(value) do
        case Map.get(list, :action) do # remember: list is a map!!!!111
          nil -> nil # why does this break sometimes?
          val -> Map.put(list, val, true)
        end
      else
        list
      end
    end
  end
end

If necessary, put longer, more descriptive comments on their own line rather than at the end of a line of code. ^[link]

Refactoring Opportunities

Never nest if, unless, and case more than 1 time. If your logic demands it, spread it over multiple functions. ^[link]

# preferred way
defp perform_task(false, hash, config) do
  nil
end
defp perform_task(true, hash, config) do
  hash
  |> Map.get(:action)
  |> perform_action(config)
end

defp perform_action(nil, _config) do
  nil
end
defp perform_action(:create, _config) do
  # ...
end
defp perform_action(:delete, config) do
  if config[:id] do
    # ...
  else
    # ...
  end
end

# NOT okay - rule of thumb: it starts to hurt at 3 levels of nesting
defp perform_task(valid, hash, config) do
  if valid do
    case Map.get(hash, :action) do
      :create ->
        # ...
      :delete ->
        if sid do   # <-- we reach three levels of nesting here :(
          # ...
        else
          # ...
        end
      nil ->
        nil
    end
  end
end

Never use unless with else. Rewrite these with if, putting the positive case first. ^[link]

# without an else block:
unless allowed? do
  raise "Not allowed!"
end

# preferred way to "add" an `else` block here: rewrite using `if`
if allowed? do
  proceed_as_planned
else
  raise "Not allowed!"
end

# NOT okay
unless allowed? do
  raise "Not allowed!"
else
  proceed_as_planned
end

Never use unless with a negated expression as condition. Rewrite with if. ^[link]

# preferred way
if allowed? do
  proceed_as_planned
end

# NOT okay - rewrite using `if`
unless !allowed? do
  proceed_as_planned
end

Always use __MODULE__ when referencing the current module. ^[link]

Software Design

Use FIXME: comments to mark issues/bugs inside your code. ^[link]

defmodule MyApp do
  # FIXME: this breaks for x > 1000
  def calculate(x) do
    # ...
  end
end

Use TODO: comments to plan changes to your code. ^[link]

defmodule MyApp do
  # TODO: rename into something more clear
  def generic_function_name do
    # ...
  end
end

This way tools have a chance to find and report both FIXME: and TODO: comments.

When developing applications, try to alias all used modules. This improves readability and makes it easier to reason about the dependencies of a module inside your project. There are obvious exceptions for modules from Elixir's stdlib (e.g. IO.ANSI) or if your submodule has a name identical to an existing name (e.g. don't alias YourProject.List because that would override List). Like most other points in this guide, this is just a suggestion, not a strict rule. ^[link]
```
# While this is completely fine:

defmodule Test do
  def something do
    MyApp.External.TwitterAPI.search(...)
  end
end

# ... you might want to refactor it to look like this:

defmodule Test do
  alias MyApp.External.TwitterAPI

  def something do
    TwitterAPI.search(...)
  end
end
```
The thinking behind this is that you can see the dependencies of your module at a glance. So if you are attempting to build a medium to large project, this can help you to get your boundaries/layers/contracts right.

Pitfalls

Never leave a call to IEx.pry in production code. ^[link]
Be wary of calls to IO.inspect in production code. If you want to actually log useful information for later debugging, use a combination of Logger and &inspect/1 instead. ^[link]
Conditionals should never contain an expression that always evaluates to the same value (such as true, false, x == x). They are most likely leftovers from a debugging session. ^[link]
Be wary of naming variables and functions the same as functions defined in Kernel, especially in cases where the function has arity 0. ^[link]
Be wary of naming modules the same as modules in the stdlib. Sometimes YourProject.DataTypeString is a less error-prone choice as the seemingly cleaner YourProject.DataType.String because aliasing the latter in a module makes the normal String module unavailable. ^[link]

Above all else

Follow your instincts. Write coherent code by applying a consistent style.

License

This work is licensed under the CC BY 4.0 license.

rrrene/elixir-style-guide