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

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
4 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.

4 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

You can write plugins for the formatter. For example, a trailing comma can be added by the freedom formatter.

It is kind of annoying committing to a library that uses their own personal style as it is just another thing to think about that you don’t normally have to bother with.

3 Likes

I’m relatively new to Elixir but encountered some similar frustration with multi-line formatting at the beginning, though I’ve since gotten more used to it. I realized, however, that I was often overmatching in order to reduce line count at the expense of readability.

The standard I’ve started to stick to is to only match in the function head on the minimum amount necessary to make that function body applicable to the given arguments. Everything else can be destructured in the function body. This means more lines of code on average, but function heads become more readable and convey more information by highlighting the specific elements of arguments that they’re really matching on.

4 Likes

That first formatted function head actually made me slightly queasy :rofl:

While we’re somewhat on the topic… has there ever been any public discussion of Elixir adopting JavaScript-style destructuring for atom keys in maps? %{foo} equivalent to %{foo: foo}. At a glance I can’t see a conflict with existing syntax.

I suppose there may be a conflict with the construction syntax %{foo | bar: “bar”} which could be problematic, but I don’t think the above is allowed without the pipe.

Many. Before and after this blog post: A story of regret and retiring a library from Hex – Andrea Leopardi

2 Likes

Thanks for the context and history!

Here, I’m just curious about something. I wonder wether there is a performance gain between getting “sub-parameters” in a function head instead of doing it inside its body.

And more specifically:

a = param.a
b = param.b

vs

%{a: a, b: b} = param

Well this is not really about the head or the body of the function…

Exactly when we are working with other people, I think we have to follow the same rules in formating the code.

You could benchmark it, but I strongly suspect no measurable difference in 99.9% of cases that Elixir users care about. You’d theoretically be doing fewer lookups and nil/membership comparisons with access in the body, but I wouldn’t be surprised if the Erlang compiler ends up “pre-loading” the data you’re going to be using in function bodies anyways. (I’m not an expert here at all, I just know enough to know that I know nothing about compiler optimization tricks that can be pulled to make stuff fast.)

1 Like

Not using a single pipe is something that was beat into me. While hurts readability in some cases, I never really found a big problem with it, yet here we are. I do find in some cases it makes sense, like I have some factory helpers like this:

build(:article)
|> mark_published()

which aren’t so nice to read like this:

mark_published(build(:article))

so I end up writing:

:article
|> build()
|> mark_published()

which isn’t the worst but there is something odd-looking about it and it appeases credo and my inherited distaste for single pipes.

It also works with the pro-trailing comma argument for diffs:

def changeset(entity, attrs) do
  cast(entity, attrs, [:one, :two, :three])
end

Now if I want to pipe on a validation at a later date, the diff is a bit more confusing than if I had just originally had:

def changeset(entity, attrs) do
  entity
  |> cast(attrs, [:one, :two, :three])
end

Anyway, I’m getting on a bit of tangent here. Regardless of all that, I’m very pro-community standards and follow those I don’t necessarily like.

2 Likes