functions with long guard clause

[ins429]

There's no guide for a function which has a long guard clause. How Ecto project is doing:

  def escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
      when is_atom(var) and is_atom(context) and is_atom(field) do
    {escape_field(var, field, vars), params}
  end

[eksperimental]

personally I would go with

  def escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
  when is_atom(var) and is_atom(context) and is_atom(field) 
  and is_atom(:even_more) and is_atom(:more) do
    {escape_field(var, field, vars), params}
  end

The reason why is because def when and and are all the same level originally, so when i split them in several lines, they should still be in the "group"

[asaaki]

Example from Elixir source (history.ex#L14-L16):

  def nth(%{queue: q, size: size, start: start}, n)
    when n - start >= 0 and n - start < size,
    do: get_nth(q, n - start)

which is a mid-long case, but I thought it's also interesting to point to.

Another approach would be to indent at function name (process.ex#L196-L200):

  def sleep(timeout)
      when is_integer(timeout) and timeout >= 0
      when timeout == :infinity do
    receive after: (timeout -> :ok)
  end

Not sure if it is nice(r).

At least those would be the two cases for single-line dos as well as functions with a normal body (block).

I think @eksperimental 's approach is also nice.

[christopheradams]

We should also consider the option that the style guide not take a position at all (beyond be consistent), since there doesn't seem to be consensus, and none of the suggestions are incontrovertibly superior.

That said, I would personally favor the suggestion from @eksperimental, for the additional reason that it does not require any new indentation rules for an IDE auto-formatter to handle. It wins on simplicity.

[eksperimental]

I totally agree with @christopheradams, and I persoalyl think that in this case the guide can give a suggestion (so users can pick up different styles and choose the one that suits their taste more), but make it clear that it is not a convention.

[dotdotdotpaul]

On Tue, Apr 12, 2016 at 2:05 AM, Christoph Grabo notifications@github.com wrote:

Another approach would be to indent at function name (process.ex#L196-L200) https://github.com/elixir-lang/elixir/blob/e1a1065ee9b389f4fdf0b9b2fad4700be61d43aa/lib/elixir/lib/process.ex#L196-L200 :

def sleep(timeout) when is_integer(timeout) and timeout >= 0 when timeout == :infinity do receive after: (timeout -> :ok) end

Not sure if it is nice(r).

I would +1 this suggestion, it's the one I will likely use.

I don't like the "outdented" version because it makes the "def" keyword hard to see when visually scanning through the code.

I don't like the 2-space indent version because then it makes the guard clause "blend in" with the function body itself, makes it harder to identify where the function ACTUALLY starts.

...Paul

[eksperimental]

Elixir code base uses the example given by the OP. The rule is when spliting a definition clause, ident with 2 levels.

@asaaki I would discourage indenting at function name. it just works because "def " because that's 4 spaces, but it souldn't with defp, defmacro, defmacrop, etc.

another thing to mention is when where to put the boolean operatorsw hen splitting guards

here's my preferred way:

  def escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
  when is_atom(var) and is_atom(context) and is_atom(field) 
  and is_atom(:even_more) and is_atom(:more) do
    {escape_field(var, field, vars), params}
  end

this is the standard used in Elixir code base

def escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
    when is_atom(var) and is_atom(context) and is_atom(field) and
      is_atom(:even_more) and is_atom(:more) and
      is_atom(:even_more) and is_atom(:more) do
  {escape_field(var, field, vars), params}
end

that means, ending the lines with the boolean operator, and increasing one indentation level the first time. Which I think is really good when writing complex guards. UPDATE: this increase in indentation doesn't seem to be part of a convention.

[migore]

I like the 4-spaces approach, it makes a clear distinction between both the def and the body of the function.

Example (@asaaki):

def nth(%{queue: q, size: size, start: start}, n)
    when n - start >= 0 and n - start < size,
    do: get_nth(q, n - start)

and (@eksperimental)

def nth(%{queue: q, size: size, start: start}, n)
when n - start >= 0 and n - start < size,
  do: get_nth(q, n - start)

I feel it's easier to scan like this:

def nth(%{queue: q, size: size, start: start}, n)
    when n - start >= 0 and n - start < size,
  do: get_nth(q, n - start)

[ryanyogan]

I have been hacking emacs A LOT lately trying to make sense of Elixir formatting and some of the larger pattern matching signatures. The only 2 cents I have is by indenting "when" that far, looking at the code would lead someone to believe they are now in the contextual scope of the function, however that is just a guard. Personally I have been switching between these two styles, I've also considered creating a macro to fix this, I have a strong feeling this syntactic sugar will change over time:

def nth(%{queue: queue, size: size, start: start}, n)
when n - start >= 0 && n - start < size,
  do: get_nth(q, n - start)

Although this may seem off, given this is FP, this reminds me more of lisp

def nth(%{queue: queue, size: size, start: start}, n) 
  when 
    n - start >= 0 && n - start < size,
  do:  
    get_nth(q, n - start)

[4ZM]

I agree with several of you that not indenting when obscures the def:s.

+1 for 2 space indent to when. Syntax highlighting will typically make when stand out and prevent confusion about where the function body starts. Optionally add a newline.

def escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
  when is_atom(var) and is_atom(context) and is_atom(field) do
  # optional empty line
  {escape_field(var, field, vars), params}
end

+1 for adding additional 2 spaces when breaking the when condition.

def escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
  when is_atom(var) and is_atom(context) and is_atom(field) and
    is_atom(:even_more) and is_atom(:more) and
    is_atom(:even_more) and is_atom(:more) do
  {escape_field(var, field, vars), params}
end

+1 for optionally putting the do on a new line with same indentation as when.

def escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
  when is_atom(var) and is_atom(context) and is_atom(field) and
    is_atom(:even_more) and is_atom(:more) and
    is_atom(:even_more) and is_atom(:more),
  do: {escape_field(var, field, vars), params}

[nathanhornby]

I'd personally like to see a suggestion make it to the guide, is there any more consensus now than when this thread was started?

I feel that there are pros and cons with all the suggestions, but the OP's seems to be the best of all worlds with the bonus that it seems to be the recommended approach.

def foobar(argument)
    when is_binary(argument) and argument in ["foo", "bar", "foobar"] do
  {:ok}
end

[eksperimental]

This is what we are doing in the Elixir project

defp escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
     when is_atom(var) and is_atom(context) and is_atom(field) and
          is_atom(:even_more) and is_atom(:more) or
          is_atom(:even_more) and is_atom(:more) do
  {escape_field(var, field, vars), params}
end

• We align when under function or macro name.
• If we need to break the guard, they will end with Boolean operators.
• following guard lines will be aligned under what comes after when.
• do goes in the last guard line.

[eksperimental]

@4ZM I wouldn't use do: for functions that take several lines,

[nathanhornby]

We align when to the name of the function or macro. - @eksperimental

Ah that's interesting, and I think contradicts an assumption up-thread so good to clarify.

@4ZM I wouldn't use do: for functions that take several lines

I'd think that the body of the function is the key there, not the length of the guard, no?

[eksperimental]

@nathanhornby I think for clarity i wouldn't go with that approach.

[nathanhornby]

@eksperimental Fair point. Doesn't seem the guide currently advises on this so maybe a good addition!

[eksperimental]

Additionally we could mentions @types and @specs. This is how we do it in Elixir codebase.

when we break it in the middle of a return value

@type spawn_opt :: :link | :monitor | {:priority, :low | :normal | :high} |
                   {:fullsweep_after, non_neg_integer} |
                   {:min_heap_size, non_neg_integer} |
                   {:min_bin_vheap_size, non_neg_integer}

when we break it in between definition and return value

@spec next(argv, options) ::
      {:ok, key :: atom, value :: term, argv} |
      {:invalid, String.t, String.t | nil, argv} |
      {:undefined, String.t, String.t | nil, argv} |
      {:error, argv}

[christopheradams]

@nathanhornby I do belive it's a preferred style these days to indent when and to further indent subsequent guards, either by a maximum of two spaces, or by indenting and aligning like @eksperimental shows.

It feels like it's time to update this guide with a rule on long guard clauses, as it's a very common formatting conundrum.

@eksperimental It seems the Erlang-inpsired typespec formatting in the current guide never caught on. Time to update it in favor of your style? :)

[eksperimental]

@christopheradams wow, I didn't even know they existed. Yes, if you feel it is an improvement, we should go for it.

[lambdadog]

Any updates on this being put into the actual guide? 4 spaces with further indentation of subsequent guards is my preferred formatting and I'd love to have something relatively "authoritative" to point the emacs-elixir folks to in order to maybe get them to reopen elixir-editors/emacs-elixir#237, seeing as this discussion ended on that note it seems like

[christopheradams]

@beta-phenylethylamine Yes, we could add a rule about long guard clauses. The formatter itself is as authoritative as you can get though.

Here's an example from the core library:

https://github.com/elixir-lang/elixir/blob/1d7545a180df8857a0233144568603f5beae706b/lib/elixir/lib/code/identifier.ex#L113-L117

https://github.com/elixir-lang/elixir/blob/1d7545a180df8857a0233144568603f5beae706b/lib/elixir/lib/calendar/time.ex#L122-L125

[cdeyoung]

I know smarter people than me have given this a lot of thought, but to throw out an idea I haven't seen yet (or missed if it is in this thread)... In my code, I tend to write macros to handle overly complex guard clauses, since I'm not a fan of multi-line function definitions. If, for some reason, a macro is still going to end up being more than one line long, I'll write my definition like this:

defp escape({{:., _, [{var, _, context}, field]}, _, []}, _type, params, vars, _env)
  when is_atom(var) and is_atom(context) and is_atom(field) and
    is_atom(:even_more) and is_atom(:more) or
    is_atom(:even_more) and is_atom(:more)
do
  {escape_field(var, field, vars), params}
end

I like putting the do on its own line. For me, this visually separates the function definition from the body better than having the do appear on the last line of a group of indented guard clauses.

[christopheradams]

We'll defer to the formatter on formatting functions with long guard clauses, without listing it explicitly for now.