Just a general question regarding code formatting. I ran credo earlier, and it gave me a couple of refactoring opportunities for Pipe chain should start with a raw value.
def find(attribute, value) do
result =
PackageQuery.find_by(attribute, value)
|> Repo.one
case result do
package ->
{:ok, package}
nil ->
{:error, nil}
end
end
I guess it wants me to reformat it so it looks like
def find(attribute, value) do
result =
attribute
|> PackageQuery.find_by(value)
|> Repo.one
case result do
package ->
{:ok, package}
nil ->
{:error, nil}
end
end
That doesn’t look particularly readable to me, if its a single thing to pass through then yea, I get that, but if the function takes multiple arguments, then it seems that it makes it less obvious.
So my question is, what does everyone else do? Always start a chain with a raw value, or just do what reads best? And is there any benefit to doing so?
In your case it might not as important as in other cases, because when you have pipe chained from 1 or 2 function calls, it’s easy enough to read it- even - when it’s written in “Java” style:
a = foo(bar(:first, :second))
However even on a simple example we see that using pipe chains makes code more readable:
a = :first
|> bar(:second)
|> foo()
But imagine here a pipe chained from 7 or more function calls eg. when using functions to validate the changeset.
Following credo in the project I work in, it’s not mandatory, but I treat it as a helpful tip. If I think I can omit some rules, I want to think twice, because things that are “ok” for me, sooner or later might be ugly for someone else… or me in the future
I guess my issue is more when there are multiple arguments to pass to the first function in the chain.
a = :first
|> foo(:second, :third)
|> bar()
|> baz()
a = foo(:first, :second, :third)
|> bar()
|> baz()
The first is less readable to me, especially if the what is being passed in makes sense together (like the attribute and value example above) It just seems weird to break them up like that.
If it’s a single string/number/struct/whatever then yea, that totally makes sense and reads nicely.
I totally agree, benperiton. I am always sad when I have to convert your second example into the first just to satisfy credo. I don’t want to turn off the check though because, as you said, it does improve readability for arity/1 functions.
It seems to imply a hierarchy for the parameters that may not exist. In the arity/1 case, it’s not just more readable; it makes semantic sense because it is pushing this piece of data through a series of transformations and/or supplying that as input for subsequent steps. But with the example here, separating :first from :second and :third implies that :first is the data being transformed and that :second and :third are incidentally needed for that first step. If foo was a function called sum, it would be clear that the parameters have equal weight and order is not important.
I would say that if you can imagine reordering the parameters supplied to foo and it makes no semantic difference to what the function is doing, you should not start with a raw value.
I think the pipeline is about data transformation, and a pipeline should always start with the data. When the data I want to work with is not a raw value(or variable) but needs to be retrived from a function call, I will start with the function call, even if it have arity 1.
Thanks guys, I think I’ll just stick with keeping it readable, and just ignore those recommendations from credo.
I might install elixir 1.6-dev and have a play with exfmt that @lpil is writing and see what that does with regards to the formatting. (I can’t remember if he showed it on the slides or not)
