Enum.reduce for idiots

I would actually swap the roles of your lookup and params variables. If order doesn’t matter, you could just turn lookup into a map and iterate over params to create your expected output.

lookup = [
  %{key: "foo", type: :string},
  %{key: "bar", type: :string},
  %{key: "baz", type: :string}
]
lookup = Map.new(lookup, &{&1.key, &1.type})
#=> %{"bar" => :string, "baz" => :string, "foo" => :string}

for {key, value} <- params,
    {:ok, type} = Map.fetch(lookup, to_string(key)) do
  %{key: key, value: value, type: type}
end

The answer above is very straightforward and I think should work pretty well!

If you do wanna go with reducing because the order matters, I think this should work:

Enum.reduce(lookup, [], fn %{key: key} = map, acc ->
  if Map.has_key?(params, key) do
    value = params[key]
    acc ++ [Map.put(map, :value, value)]
  else
    acc
  end
end)

Have not tested it, but I’d believe it will work

You don’t say what error you might have gotten, but in my quick reading I only see only problem “.” is for accessing fields of records, you want params[key] basically, but of course you need a conditional on whether or not it exists, so you also want to if or case on a check for its existence

You have 2 mistakes in code:

1. You are calling params.k expecting that k is variable. params.k works like params[:k] and therefore it would not work for you. Also you cannot mix Atom and String types as they are completely different types.

2. Your with clause would always raise something like:

   ** (KeyError) key :k not found in: # …
   

   The raison is described in 1st point. However mistake here is to assume that raise would simply fail with clause. Elixir way is “let it fail”. Of course you can write catch/try, but unless you have a strong reason for it it’s generally not recommend.

Here are my 2 proposals:

defmodule Example do
  def safe_sample(list, map) when is_list(list) and is_map(map) do
    list
    |> Enum.reduce([], fn %{key: string_key, type: type}, acc ->
      case Enum.find(map, &custom_has_key?(&1, string_key)) do
        {atom_key, value} -> [%{key: atom_key, type: type, value: value} | acc]
        nil -> acc
      end
    end)
    |> Enum.reverse()
  end

  defp custom_has_key?({atom_key, _value}, string_key) do
    Atom.to_string(atom_key) == string_key
  end

  def unsafe_sample(list, map) when is_list(list) and is_map(map) do
    list
    |> Enum.reduce([], fn %{key: string_key, type: type}, acc ->
      atom_key = String.to_atom(string_key)

      case map do
        %{^atom_key => value} -> [%{key: atom_key, type: type, value: value} | acc]
        _ -> acc
      end
    end)
    |> Enum.reverse()
  end
end

There are 2 major differences between safe and unsafe versions:

1. safe is good for data from untrusted source like user input
2. unsafe is faster because we have instead of Enum.find/2 we are using pure pattern matching which is optimized by compiler

In case we are sure about existence of string_key anywhere in params (as Atom) we can use String.to_existing_atom/1 which is safe, but in case like yours it would raise ArgumentError and that’s why I have not provided a safe way with optimized pattern-matching usage.

Code you wrote is good in theory and may be bad in practice.

You are doing something (swapping roles of 2 variables) without a discussion with author of topic. This is called assumption and in theory there is nothing bad. However take in mind that there are no 2 exactly same people and it’s only matter of time when you would expect different behaviour.

In this example you reached expected result and therefore there is nothing bad in your code now. I can see 2 cases when assumption would cause trouble:

1. On prod data - pay attention that every data posted on forum (and on source with public access) should be redacted, so the code which works here does not need to works in prod.

2. Even if assumption works in practice it may fail when enhancing original theory. Let’s take an example. Somebody asked you to make a decorations and you should use eggs for it. Mostly by assumption of easterday painting eggs is just “normal”, but if we do not want them for easterday even if they would be worth thousands of USD we may need new eggs for decorations.

This one is unfortunately a popular mistake. Using [head | tail] optimization along with Enum.reverse/1 is faster than appending two lists like in your example.

You see well, but you do not everything. You missed that there are used 2 different types (Atom and String).

Hope it helps.

My ugly one.. I’m also a fan of looping over params, and using lookup as a dictionary.

iex> Enum.reduce(params, [], fn {k, v}, acc -> 
  [%{key: k, value: v, type: Enum.find(lookup, & &1.key == to_string(k))[:type]} | acc] 
end)
[
  %{key: :foo, type: :string, value: "bananas"},
  %{key: :baz, type: :string, value: "apples"}
]

I think you shouldn’t use Enum.reduce for this. Enum.flat_map is better, since you are going from list to list and possibly omitting some.

lookup = [
    %{key: "foo", type: :string},
    %{key: "bar", type: :string},
    %{key: "baz", type: :string}
 ]

params = %{"foo" => "bananas", "baz" => "apples"}

Enum.flat_map(lookup, fn 
  map when is_map_key(params, :erlang.map_get(:key, map)) ->
    [Map.put(map, :value, params[map.key])]
  _ -> []
end)

Thanks for everybody’s help. I hadn’t thought of swapping roles and looping over the params.

But it was great to learn from all the different takes on using Enum. It’s been a real benefit, so thank you.