Underscored variables - which do you prefer, descriptive or not?

So I came from the Erlang world. Recently I got into a debate at work about underscored variables.

Half of us (including me) do:

 def foo([], _byte_count, _file_count, acc), do: acc

 def foo([node | tail], byte_count, acc) do 
  # Some code

 def bar(%{something: var} = _state) do
  # Some code 

The other half does:


 def foo([], _,  _, acc), do: acc

 def foo([node | tail], byte_count, acc) do 
  # Some code

 def bar(%{something: var})do
  # Some code 

What’s the preference?

6 Likes

I think (a) is better, but I do (b) because I’m lazy.
As longs it’s consistent (over the codebase - use credo), both are OK I think.

4 Likes

I prefer (a) because it saves time.

7 Likes

having done both a and b, now it’s a from now on. b is great, until you come back to the code after a while. @cmo is right, imo.

7 Likes

I do a, however sometime go for b when I have a function with multiple heads and have to repeat a lot of the same stuff.
For example:

def handle_event(event, unsigned_params, socket)

def handle_event("validate", %{"some" => value}, socket) do
  ...
end

def handle_event("save", %{"some" => value}, socket) do
  ...
end

def handle_event("refresh", _, socket) do
  ...
end

def handle_event(_, _, socket) do
  ...
end

...
6 Likes

Agreed with @tomkonidas! Generally if I have 3 or more clauses, I’ll use a function head and then refer to skipped arguments with _.

The exception to that, however, might be very commonly-used callbacks like handle_event/handle_info in LiveViews. Basically if I’m implementing a well-known and oft-used behaviour, I’m more okay using _.

If the function is not a callback, though, I definitely want to see written-out params somewhere. It can be very frustrating to read code that looks like:

def do_the_thing(nil, {_, false}, _, _, meta) do
  ...
end

def do_the_thing([], {true, _}, extra_cool_param, _, meta) do
  ...
end

def do_the_thing([item | rest], _, _, fun, _) do
  ...
end

...

I really enjoy reading through library code when I need to better understand something, and patterns like the above are rather common once you get pretty far “into” a library and are dealing with some kind of under-the-hood data transformation/reduction.

1 Like

I do A. I always want to know what the var is, not now but six months from now when I revisit this part of my codebase. Super important to me

2 Likes

One argument for (b) is that it should be clear what it does from the function heads that are using it. And for the ones that aren’t using it you don’t care what it is. If you are trying understand the function as a whole there are function heads, docs and type specs.

1 Like

Seconding this. At best naming an unused variable is a tradeoff. Cost of noise/cognitive load for benefit of reference, but as mentioned there are better places to use as reference. And as was also mentioned, either way you will want to be consistent, and consistently naming all unused vars ends up being a lot.

I think if a function is hard to read because it has a lot of unused variables, that could possibly be considered a code smell. And if refactoring really isn’t possible, function docs are the proper way to justify the tech debt.

2 Likes

I always use (a) because as others already said at one point you revisit the code and are wondering what the hell is the ignored variable. Happens much more often than our present lazy self wants to believe. If I don’t feel like using (a) then I stand up from the computer – obviously I shouldn’t be coding in that mind state!

That being said, (b) is useful when you need a catch-all clause but only if you already exhaustively covered all other cases.

1 Like

Always a) except in pattern catch-alls where there is no obvious name. But even then I should still do a). Just don’t call it _other .

I would declare @spec and in most cases the function head with meaningful argument names. Then I’d avoid noise in function definitions themselves.

@doc """
Recursively iterates list of nodes, amending the accumulator given as a third parameter
  according to `byte_count` value.
"""
@spec foo([node()], non_neg_integer(), list()) :: list()
def foo(nodes, byte_count, acc)
   
def foo([node | tail], byte_count, acc), do: # Some code
def foo(_, _,  acc), do: acc
2 Likes

What about for private and anonymous functions?

The only thing to be aware of the that the _var are just normal variables so if you use the same one in the function head then you are saying that they must be equal. You can in fact use them as normal variables. Note the compiler will warn you when you do this but be careful.

The _ variable is the only special variable as it always matches and is never bound.

12 Likes

Private functions still deserve the spec. Anonymous functions usually need all the arguments or they are obvious.

2 Likes

I don’t really care as long as what’s going on is relatively clear. We often use @specs in public functions which should encode all the possibilities which is often the better source of truth assuming dialyzer is actually run regularly. For private functions w/o spec though, doing _ or _name is pretty meaningless if you can get the context by looking up/down. The only case where I can see this being annoying is in a PR where the diff doesn’t immediately give full context. I’d argue for application code changes to be reviewed in a text editor where you can bounce around the code anyway, for meaningful changes. Ultimately more context is usually better than less.

_ to me more explicitly says, “we don’t care about this at all and intend to never care”, which means I shouldn’t waste my time thinking about the possibilities for _name, which I tend to appreciate.

The bike shed is a now a vicious violet! Also what Robert said about _ being special was a neat TIL, thanks for that!

Also a TIL for me in that underscore prefixed variables are actually bound (I just played around with it in iex).

Is that actually something that some living code relies on, or is this something Elixir could take the stand on and treat _foobar identical to _?—would that cause problems? The two reasons that come to mind are 1) existing library code (which is immune to warnings_as_errors) and 2) function definitions beginning with an underscore, though I feel those are treated completely differently by the compiler… I’m not actually sure, of course, so am I wrong?

I otherwise can’t imagine a scenario where you should want to match with binding underscores, but maybe a situation like this could be clearer with the underscore:

def foo(%User{id: _same}, %Post{user_id: _same}), do: # ...

…but again, we’re getting the warning anyway and I don’t necessarily agree that this is clearer (and I usually go for the when version anyway even though I think the former is pretty sexy).

Annnnnnyway, In all the time I’ve been annotating my throwaway matches, I’ve never even had the clue that they were actually being bound, so I’m just really curious if this seemingly hidden gotcha could be eliminated without much friction.

1 Like

So the special handling of the _ variable is defined in the Erlang language while the handling of _name variables is all done by the Erlang compiler. It is where the warnings around the _name variables come from but it quite happily just passes them on.

So if Elixir is to change how the _name variables are to be treated it could cause some confusion. It would just have to be careful how it passes them on.

3 Likes

I also do both. Typically, I prefer self-documenting code, so _name is going to be my default mode, but when you’re grouping function heads correctly, in some cases, like in the catch-all matches at the end of a logical group, names no longer matter. The last clause of a recursion is a simple example where I will throw away ignored accumulators and just leave a single underscore in their stead.

I take mostly from column A with a little bit of column B.

2 Likes

always a :slight_smile: