Obsession with line length and hideous formatting (am I writing elixir wrong?)

What do y’all think about the defaults of the mix formatter? I often find long lines being being turned into multiple breaks with weird indentation. The output is so often hideous and completely unreadable.

This happens almost exclusively in long function definitions with pattern matching. Since pattern matching function parameters is a core part of elixir and incidentally very wordy, it doesn’t make sense for formatters to make such a fuss over line length. It’s making me doubt whether I’m writing elixir in the intended manner.

Look at the example below. It’s almost impossible to visually separate the function definition from the first line of the function.

This version is objectively easier to read, but apparently wrong according to the flagship formatter. Is 103 columns really that sinful for a function definition?

Line length is one of the few things you can configure on the formatter. If you feel the default is to short increase it.

4 Likes

Thanks, that’s not actually in the hex docs so I wasn’t aware. That should do for now!

Personally, 9 out of 10 times I see a function clause that spans multiple lines as a code smell. Ask yourself:

  • Do you need all of that pattern matching in the function head?
  • Can you match some in the body instead?
  • Can you write a private guard to reduce matching or guard clauses?
  • Can you use a case within a function (or all compiles down to a case anyhow)?
3 Likes

Agreed. I’ve come across “over-matching” to the point where it takes a second to figure out which matches are required for flow control.

handle_event is pretty ubiquitous so it’s not as big a deal, but I always write it like this:

def handle_event("save", %{"profile" => profile_params}, socket) do
  profile = socket.assigns.profile
  # ...
end

I feel that setting profile in the body is a lot less noisy than nested destructuring, especially when combined with capturing the whole value. My other reasoning is that the event name and param arguments are specific to the function itself whereas profile already exists in state, so destructuring it the head isn’t telling me anything new—I already know that I always have access to profile if I have a socket.

It’s all a matter of taste, of course, and totally up to you!

4 Likes

After playing with it a while, I really want to configure the formatter to never breakdown a long function head. I like how it formats other lines, so changing the line length breaks that preference. Is this possible?

Are you able to give examples of the last two in alleviating this issue? Thanks!

Thanks for the kick, I think this might be a case of me being new to the language. I find myself doing the same thing with “over-piping” the socket struct instead of making a regular function call with the socket as first param.

{:noreply, socket |> assign(:other, "other")}

# instead of

{:noreply, assign(socket, :other, "other")}
2 Likes

Think you mean subjective :slight_smile: I actually find the formatted version easier to read. It’s easier for me to scan vertically than horizontally and I don’t like long lines. I tend to think one can get used to just about any formatting if you look at it enough, but also agree with others that too many matches in a function head can be hard to parse.

6 Likes

Sure thing!

  1. Here’s an example of writing a private guard from Oban: oban/lib at main · sorentwo/oban · GitHub
  2. Here’s an example of reducing the number of function clauses and minimizing the head size by matching with a case in the function body (it is from Oban Pro, so no link)
  def job_to_key(%{worker: worker, args: args}, partition) do
    case partition do
      %{fields: ["worker"], keys: []} ->
        with_hash({worker, nil})

      %{fields: ["args"], keys: keys} ->
        with_hash({nil, Map.take(args, keys)})

      %{fields: [_, _], keys: keys} ->
        with_hash({worker, Map.take(args, keys)})

      _ ->
        with_hash({nil, nil})
    end
  end
3 Likes

The formatter is something that I have a bittersweet feeling about and it’s perhaps the tool that I least like in the ecosystem. It’s great to keep a codebase consistent across teams if you don’t really want to think about it.

I hope for more configuration options in the future while keeping some opinionated default because I really care how my code reads. I also got my code butchered by the formatter a lot in the past because it doesn’t care about nuances as I do.

TLDR: It’s great for standardization, but the idea that a tool would decide how your code reads better than yourself is nuts to me.

There’s some value in those considerations overall if you want to increase readability in some cases, but I disagree with the premise of considering it as code smells. Mainly because there’s no real underlying issue in your code if you don’t follow this. It’s all a matter of preference on how you want to read your code. BTW, extracting code elsewhere is not always preferred for legibility.

That’s exactly my point, thanks @baldwindavid! Legibility is something really subjective and I think the main gripe some people might have with the formatter is that it’s not really flexible in the first place. It actually enforces very opinionated defaults which are also very subjective.

3 Likes

It’s a code smell because typically when you overload a function head, it masks the critical part of what you’re matching on. Code is about communication; names, whitespace, and line lengths matter.

Of course, this is all subjective—I admit to having strong preferences—and sometimes it is necessary to format across multiple lines; hence that 1 out of 10 loophole.

2 Likes

Code is about communication; names, whitespace, and line lengths matter.

No argument here, but I wouldn’t go as far as saying it “masks” anything critical because is totally your perception about it, and I respect that. My point is that because it’s subjective, I don’t think we can quite classify it as a code smell so objectively as you did :blush::muscle:.

1 Like

To me one of the big benefits of the formatter is that it has so few options. A good chunk of elixir dependencies uses the formatter, so there’s no difference between looking at code I wrote or some third party code. Yes, to get to “one way of formatting” one has to make subjective decisions once, but when they’re taken there’s no need to discuss them further. There are some cases where the formatter doesn’t produce optimal code, but many times I’ve found ways to make it work. Given those are the 1% I’m happy to make those changes to get the benefits for the other 99%.

9 Likes

@LostKobrakai You know that 80% of statistics are false right!? haha :sweat_smile:

When talking about code I don’t like compulsory standards - that’s one of the reasons I would not touch python and other languages with similar mantras like: “There should be one - and preferably only one - obvious way to do it” .

IMHO, this is a double-edged sword and I wonder how much we are sacrificing in some cases by treating it as “one size fits all” - which I don’t think it’s just 1% - not in my experience btw; or perhaps I’m finding myself in your 1% too much :grimacing:.

In the end, there’s no silver bullet, but I think this could still be achieved if instead of having no options, we could have opinionated defaults for the majority of cases as well as the ability to change them.

Yes, there are no silver bullets, but only tradeoffs. But requesting changes to the tradeoffs made on the existing formatter won’t necessarily make it more of a silver bullet as well. It will only make it more usable tool for some people by trading in the usefulness others see in the tool.

1 Like

Yes, true and true. The thing is, allowing people more freedom to configure the formatter rules won’t cancel out the benefits we already have. My point is that standards should be opt-in.

Since we are already not required to use the formatter at all, I think it wouldn’t hurt to make it more configurable overall. Generally speaking, there will never be an amount of configuration that fits 100% of all the cases as well as lack thereof.

As you said, there are only tradeoffs.

FWIW, there’s a Credo check for that exact pattern - Credo.Check.Readability.SinglePipe

1 Like

Did you intend to link directly to a private guard?

I did, that’s strange. Here’s the link again: oban/oban.ex at main · sorentwo/oban · GitHub