Collectable for error tuple, a good idea?

Questions / Help
1

I often find myself with code like this when applying a function that returns error tuples to an enumerable:

is_ok? = fn x -> 
  if x < 5, do: {:ok, x}, else: {:error, "too big"} 
end

Enum.reduce_while(1..5, {:ok, []}, fn v, {:ok, vs} ->
 case is_ok?.(v) do
  {:ok, v} -> {:cont, {:ok, [v | vs]}}
  {:error, _} = e -> {:halt, e}
 end
end)

But, if I write a simple collectable implementation for Tuple, I can write this instead:

for x < - 1..5, into: {:ok, []}, do: is_ok.(x)

I much prefer the second: it’s short, it’s clear, and I don’t mind too much that it iterates the whole enumerable every time. Is implementing a protocol for convenience on a built in type like this a good long term decision?

2

If you implement collectable and you do

for x < - 1..1000000, into: {:ok, []}, do: is_ok.(x)

You will perform 999995 unnecessary operations

3

Your first code stops at the first error. Your second code doesn’t.

4

I much, much prefer the first version. I can immediately see what happens and understand.

I always prefer clarity over brevity. Always. Every. Single. Time.

4 Likes
5

Then you’d better change the accumulator, I guess

Enum.reduce(1..5, %{ok: [], errors: []}, fn v, vs ->
  key = if is_ok?.(v), do: :ok, else: :error
  Map.update!(vs, key, &[v | &1]}}
end)
6

How should this chunk behave? For me(if you implement `Collectable` protocol), the result should be something like:

{:ok, [], {:ok, :v1}, {:error, :reason}, ...}
7

My reading was that the Collectable implementation would check the shape of the block’s return value:

  • if it’s {:ok, value} and the accumulator is {:ok, values} then the new accumulator is {:ok, values ++ [value]} (or equivalent with reverse-at-the-end for performance)
  • if it’s {:ok, value} and the accumulator is anything else then the new accumulator is unchanged
  • if it’s {:error, e} then the new accumulator is {:error, e}

I don’t think I’d personally choose this approach, since it requires defining Collectable for all tuples but only actually cares about specific shapes

2 Likes
8

That is how I wrote the collectable implementation, and it would also raise an exception if it was given a tuple with an unexpected shape. Ultimately I agree with you and @FlyingNoodle that using a protocol isn’t a good approach.

I ended up creating a Result stuct with a collectable implementation with similar behavior.

@mudasobwa I should have been more clear in my examples. I used Enum.reduce_while/3 to illustrate that the desired outcome was that the first error encountered would be the result for the collectable.

9

Yeah, doesn’t seem like Collectable any more, significantly different from other implementations.