Step away from the pipe

Chat / Discussions
1

I recently wrote some code

defp apply_result_parse(cmd,result_map, attribute_map) do
  { tag, parse_f } = Map.get(attribute_map, cmd)
   result = Map.get(result_map, cmd)
   value = parse_f.(result)
   %{tag => value}
  end

And my first thought was, “How can I write this as a pipe?”
And while it’s obvious that you can get rid of the tmp variables result
and value with this code.

 defp apply_result_parse(cmd,result_map, attribute_map) do
   { tag, parse_f } = Map.get(attribute_map, cmd)
   
   %{tag => result_map
                  |> Map.get(cmd) 
                  |> parse_f.() }
 end

But I find the first version a whole lot more readable. As I do more and more elixir,
I become more and more suspicious of pipes. They do make some things much more
straight forward, but I think they can just as easily obsfucate code.

Am I just falling into my default retro-grouch thinking?

2

What about

 defp apply_result_parse(cmd, result_map, attribute_map) do
   { tag, parse_f } = Map.get(attribute_map, cmd)
   
   %{tag => result_map |> Map.get(cmd) |> parse_f.() }
 end

or

 defp apply_result_parse(cmd, result_map, attribute_map) do
   { tag, parse_f } = Map.get(attribute_map, cmd)

   result = 
     result_map 
     |> Map.get(cmd) 
     |> parse_f.()

   %{tag => result}
 end

(this would probably the version I’d write)

?

Sometimes it is easier to read left-to-right, and sometimes it is easier to read right-to left (and sometimes infix is easier, as is the case when doing mathematical operations).
I’d say it is a personal preference of how to write this snippet.
Personally I find all four versions of this code to be quite readable. :slight_smile:

3 Likes
3

I like the first version by @Qqwy. It’s quite clear and readable. In this case perhaps I’d replace pipeline with a staircase, but it doesn’t matter much.

I find the first version you wrote quite hard to read. After 2 lines in the code you have three temp vars and it’s still not clear what do you want to do with them. I need to enter the “manual interpreter” mode here, carefully reading each line, keeping the stack in my head before it all ties up.

@Qqwy’s example ties up in the second line with only two temp vars. It’s kind of like he gives a spoiler alert more quickly :slight_smile:

Alternatively, you could consider something like (untested):

defp apply_result_parse(cmd, result_map, attribute_map) do
  {tag, parse_f} = Map.get(attribute_map, cmd)
  %{tag => apply_function(result_map, cmd, parse_f)}
end

defp apply_function(result_map, cmd, parse_f), do:
  parse_f.(Map.get(result_map, cmd))

As an aside, consider using Map.fetch!, since it seems you expect the value to exist, at least in the attribute_map. If you do want get/2, then you might clean it up a bit with []:

defp apply_result_parse(cmd, result_map, attribute_map) do
  {tag, parse_f} = Map.fetch!(attribute_map, cmd)
  %{tag => apply_function(result_map, cmd, parse_f)}
end

defp apply_function(result_map, cmd, parse_f), do:
  parse_f.(result_map[cmd])
3 Likes