Arguable advice from `Credo.Check.Refactor.AppendSingleItem`
Environment
- Credo version:
1.7.5-ref.main.81cc50fba+uncommittedchanges - Erlang/Elixir version:
Erlang/OTP 26 [erts-14.1] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit] Elixir 1.15.6 (compiled with Erlang/OTP 26) - Operating system: OSX
The issue
The Credo.Check.Refactor.AppendSingleItem correctly flags a bunch of places where singles items are in fact appended to lists and that's all good.
Example from credo list = list_so_far ++ [new_item]
The issue is that the example states to use Enum.reverse/1 when order matters and gives the following adivce:
list = [new_item] ++ list_so_far
# ...
Enum.reverse(list)
The thing is that the given solution is somewhat wrong. If order matters and the list_so_far is something like [1, 2, 3] and we try to append 4 the resulting list looks like this[3, 2, 1, 4].
The correct solutions would be:
list = [new_item] ++ Enum.reverse(list_so_far)
# ...
Enum.reverse(list)
But I would argue that ist in fact messier than before.
Additionally
To add to that two possible functions come to mind to get around using ++ there would be
-
Enum.concat(list_so_far, [new_item])or ... -
List.insert_at(list_so_far, -1, new_item)
the thing is both are using the ++ themself. See Enum.concat and List.insert_at
While I think that I am getting what you are trying to say, I am not sure what you suggest to do about it.
I understand why you are saying that the example given might be misunderstood. Yet, this hasn't come up in the 7 years this check exists.
If you have a suggestion for a better phrasing of the check's description, please share! ✌️