Skip to content

Conversation

@wojtekmach
Copy link
Member

No description provided.

@josevalim
Copy link
Member

We should probably then add drop! for consistency.

@wojtekmach wojtekmach changed the title Add Map.take!/2 and Keyword.take!/2 Add take!/2 and drop!/2 to Map and Keyword Oct 31, 2025
Comment on lines +1318 to +1331
def take!(keywords, keys) when is_list(keywords) and is_list(keys) do
allowed_keys = Keyword.keys(keywords)

:lists.foreach(
fn key ->
if not :lists.member(key, allowed_keys) do
raise KeyError, key: key, term: keywords
end
end,
keys
)

:lists.filter(fn {k, _} -> :lists.member(k, keys) end, keywords)
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not efficient. To limit traversals, one idea I had was:

def take!(keywords, keys) when is_list(keywords) and is_list(keys) do
  take!(keywords, MapSet.new(keys), MapSet.new(), _acc = [])
catch
  key ->
    raise KeyError, key: key, term: keywords
end

def take!([{key, value} | rest], keys, used_keys, acc) do
  if MapSet.member?(keys, key) do
    take!(rest, keys, MapSet.put(used_keys, key), [{key, value} | acc])
  else
    take!(rest, keys, MapSet.put(used_keys, key), acc)
  end
end

def take!([], keys, used_keys, acc) do
  case MapSet.to_list(MapSet.difference(keys, used_keys)) do
    [] ->
      :lists.reverse(acc)

    [key | _] ->
      throw(key)
  end
end

but I haven't benchmarked it yet.

Copy link
Member

@josevalim josevalim Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simplest imlpementation is probably one that tracks which keys have been found and which ones haven't. Something like this:

{taken, not_found, _found} =
  Enum.reduce(kw, {[], keys, []}, fn {key, value}, {acc, not_found, found} ->
    case pop_member(not_found, key, []) do
      {:ok, not_found} -> {[{key, value} | acc], not_found, [key | found]}
      :error ->
        case :lists.member(key, found) do
          true -> {[{key, value} | acc], not_found, found}
          false -> {acc, not_found, found}
        end
    end
  end)

if not_found != [] do
  raise key not found
end

:lists.reverse(taken)

where:

defp pop_member([key | rest], key, acc), do: {:ok, rest ++ acc}
defp pop_member([head | rest], key, acc), do: pop_member(rest, key, acc)
defp pop_member([], _key, _acc), do: :error

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the same -- Instead of raising key not found, it would be great to return found and not found keys.

@josevalim
Copy link
Member

I will close this one for now because we cannot actually type this function (and most type systems used at scale can't type it either):

  1. at best, we can say it returns a map that may have the keys given to take (which is counter intuitive to the role of take!)
  2. set-theoretic types could type this if we change the implementation of lists but that has other consequences

I will write a blog post on the topic and publish it either on dashbit.co or elixir-lang.org.

@josevalim josevalim closed this Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants