Having trouble with `formatter` and `case`

Questions / Help
, ,
1

Hello! I am having issues with the Elixir formatter and a long case expression. What’s happening is that this:

defmodule Test do
  def evaluate_contract(%{from: from, to: to, value: value} = t, contract) do
    case contract do
      x when is_number(x) -> x
      s when is_binary(s) -> s
      [] -> true
      {} -> true
      true -> true
      false -> false
      {:if, co, tr, fa} -> if evaluate_contract(t, co), do: evaluate_contract(t, tr), else: evaluate_contract(t, fa)
      {:+, left, right} -> evaluate_contract(t, left) + evaluate_contract(t, right)
      {:*, left, right} -> evaluate_contract(t, left) * evaluate_contract(t, right)
      {:-, left, right} -> evaluate_contract(t, left) - evaluate_contract(t, right)
      {:==, left, right} -> evaluate_contract(t, left) == evaluate_contract(t, right)
      {:>, left, right} -> evaluate_contract(t, left) > evaluate_contract(t, right)
      {:<, left, right} -> evaluate_contract(t, left) < evaluate_contract(t, right)
      {:and, left, right} -> evaluate_contract(t, left) and evaluate_contract(t, right)
      {:or, left, right} -> evaluate_contract(t, left) or evaluate_contract(t, right)
      :from -> from
      :to -> to
      :value -> value
      _ -> false
    end
  end
end

gets formatted to:

defmodule Test do
  def evaluate_contract(%{from: from, to: to, value: value} = t, contract) do
    case contract do
      x when is_number(x) ->
        x

      s when is_binary(s) ->
        s

      [] ->
        true

      {} ->
        true

      true ->
        true

      false ->
        false

      {:if, co, tr, fa} ->
        if evaluate_contract(t, co), do: evaluate_contract(t, tr), else: evaluate_contract(t, fa)

      {:+, left, right} ->
        evaluate_contract(t, left) + evaluate_contract(t, right)

      {:*, left, right} ->
        evaluate_contract(t, left) * evaluate_contract(t, right)

      {:-, left, right} ->
        evaluate_contract(t, left) - evaluate_contract(t, right)

      {:==, left, right} ->
        evaluate_contract(t, left) == evaluate_contract(t, right)

      {:>, left, right} ->
        evaluate_contract(t, left) > evaluate_contract(t, right)

      {:<, left, right} ->
        evaluate_contract(t, left) < evaluate_contract(t, right)

      {:and, left, right} ->
        evaluate_contract(t, left) and evaluate_contract(t, right)

      {:or, left, right} ->
        evaluate_contract(t, left) or evaluate_contract(t, right)

      :from ->
        from

      :to ->
        to

      :value ->
        value

      _ ->
        false
    end
  end
end

You can try this with the online formatter. I think this is a pretty drastic format and greatly reduces the function’s readability. It also triples the case expression’s body.

I expected the if expression to get expanded to multiple lines but not have every other case expanded to multiple lines. I think after expanding the if, the formatter could put spaces between the clauses (I wouldn’t), but I don’t see the reason why it puts all the short match results on a new line just because of one long result expression.

Is this expected? If so, does anyone know how to configure or any existing custom plugins that work a little better for case? One option I have is to simply shorten the function name. Although not an ideal change due to formatting, that may be the easiest solution.

2

This is expected behaviour to keep formatting standard and predictable. If one clause is multi line all clauses are made multi line.

Breaking out the long lines into separate functions to keep the case clauses short is the right way. It’ll also make the code more understandable by using descriptive names.

elixir_style_guide#multiline-case-clauses

1 Like
3

Ahh, that’s too bad. :frowning:

to keep formatting standard and predictable

I think it’s better to say the formatter does it that way because it wants to. :slight_smile: There’s nothing unpredictable or nonstandard about just creating a multi-line clause for clauses that are too long but leaving the others alone. In fact, compared to other languages that have such syntax, such as F#'s match or OCaml, it is non-standard to have a multiline clause force all others to not only be multiline but have a newline space between them.

In my opinion, this is the wrong choice by the formatter. It makes functions less readable, longer, and triples the newlines with no benefit. What’s interesting is that the formatter doesn’t even breakout the if expression into multilines, but I can force that by doing it myself. In fact, I originally did that myself before the recursive calls were in place, as I don’t like single line if expressions, and that’s when I found out about this formatter behavior. It was an even worse change then (without the recursive calls).

Breaking out the long lines into separate functions to keep the case clauses short is the right way.

I’m not sure I understand. Creating one-off helper functions for individual result expressions seems tedious and also distracting. Do you mean “into separate lines”?

using descriptive names.

I definitely agree, but fighting with the formatter here actually makes one prefer shorter names. I’d rather have shorter names than have to scroll to see a function definition.

elixir_style_guide#multiline-case-clauses

Ahh! It’s in the style guide! It gets worse. :frowning: :wink: I’ve never seen that in other languages. Maybe it’s from Ruby (which I haven’t used)?

I understand the need for consistency and generally have no problems with the formatter, but it would be nice for some configurability for highly opinionated, non-standard choices that the formatter makes. (The other one that comes to mind is comments in pipelines.)

For now, I have shortened the function name, and that seems to be an okay compromise. It lets me keep each clause on a single line and things readable enough where the shorter name is no issue, and maybe even preferable. (I’m porting this code from a book, so I initially follow the book’s function names.)

4

All valid opinions. It’s probably very hard to make everyone happy with standard choices and I’ve just come to accept what the formatter does because it’s predictable for me, and I can just get on with the work.

There are probably other alternatives to running mix format, something that works through beautifier or its like, which can be configured to your liking. But as far as I can tell the position taken on the formatter mirrors that of elm-format

The benefits of elm-format:

  • It makes code easier to write, because you never have to worry about minor formatting concerns while powering out new code.
  • It makes code easier to read, because there are no longer distracting minor stylistic differences between different code bases. As such, your brain can map more efficiently from source to mental model.
  • It makes code easier to maintain, because you can no longer have diffs related only to formatting; every diff necessarily involves a material change.
  • It saves your team time debating how to format things, because there is a standard tool that formats everything the same way.
  • It saves you time because you don’t have to nitpick over formatting details of your code.
5

Yea, I am in one of the stages of “formatter grief” at the moment. :laughing: Although, I do wish there was some configuration for these opinionated stances for individual orgs, teams, or projects to settle on.

For now, I’m trying to move into the acceptance stage of formatter grief. I’m also building up so-called tricks that help me strike a balance between what I’d like vs what the formatter does. I’ll paste what I came up with tomorrow, and I think it’s a fine enough compromise.

I’ll take a look at the Elm docs you sent tomorrow. Thanks for those.

1 Like
6

I think a better word here - and what @03juan actually had in mind - would be “consistency”. Granted, it’s consistency with it’s own rules which can be changed. But my take is that the fewer the rules the better.

Yes, this is the way. There are situations where no code formatter rules will make the code look good (“for most people :tm:”) unless you refactor it. For me, your code is a good example of that. Shortening the function name is one of the approaches (you can have top level evaluate_contract and then eval). Refactoring the common parts would be another:

{op, left, right} when op in @binary_ops -> evaluate_binary_op(t, left, right, op)

I’d say this is all or nothing. You need to be quite opinionated when deciding which options should be configurable.

1 Like
7

This is the way. :robot:

To add sth useful: What I’d like the formatter to do is, that when I shorten the too-long-line the cases are restored to single lines.

1 Like