Re: [elixir-core:8457] Adding `take!` and `drop!` to Map and Keyword modules

[José Valim]

Just for completeness, if we add those functions, we also need to add Map.drop! and Keyword.drop!. Btw, your description here makes more sense, as I can see this feature being useful for keywords, but not really for maps. Do you have some code snippets you could show us too?

Thanks Valter!

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

On Sun, Jan 20, 2019 at 7:36 PM Valter Sundström <valter.s...@gmail.com> wrote:

I somewhat often find myself wanting to take subsets of maps and keywords lists, verifying that all keys I take exist.

An initial PR for this is available here, but as josé pointed out, a discussion needs to be had regarding the need for it.

One of my main usages of this is for example when initializing the state of a `GenServer`, or if extracting some options from a keyword list to pass along to some other module.

Please chime in if you have any opinions!

// Valter

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/5990bdcd-83d2-4925-8af6-79bbaa488667%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Valter Sundström]

Sorry I messed up the title forgetting to add `[proposal]`, but since you answered here, let's go with this thread.

The GenServer case I am talking about is essentially this:

  def start_link(opts) do

    state = %{ 

      foo: Keyword.fetch!(opts, :foo),

      bar: Keyword.fetch!(opts, :bar)

    }

    GenServer.start_link(__MODULE__, state, opts)

  end

I believe I am not alone in doing similar things, but possibly I am. The reasoning being to fail quickly at initialization.

This could instead be written as:

  def start_link(opts) do

    GenServer.start_link(__MODULE__, Keyword.take!(opts, [:foo, :bar]), opts)

  end 

This, I think is clearer.

[José Valim]

Those are not equivalent though, as one is passing a map and the other a keyword list. You could call Map.new but it is unlikely you have a map made only of external arguments... so I am thinking you can't really escape from buiilding the map by hand.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/bf4a83a9-c925-40fe-a1f6-fbbb7159b5cb%40googlegroups.com.

[Valter Sundström]

Ah, yes, that's true, and this was perhaps a too quickly written example.

I would still prefer 

state =  opts |> Keyword.take!([:foo, :bar])|> Map.new()

But this is probably not a good enough reason to add this to the standard library.

I will try to pay attention to more convincing examples in my own usage.

In general it often has to do with redirecting options to other functions.

[Raphael Costa]

The need for this happens so often at my team that we actually defined a `take!/2` function for maps in our codebase. We use it mostly for creating a map of a subset of the keys in a struct (e.g. in order to render JSON or send it to Elasticsearch). In these cases, we want the runtime to yell at us if there's a typo in one of the key names, instead of silently passing a map without that key around. It is a very simple function to add to any codebase, so I don't think the mere fact that this function is absent is enough to justify bringing in a new dependency to my project, let alone adding it to the standard library.

However, our community always preferred (and was encouraged to) write assertive code, as it leads to errors that happen in the right place. Under this light, I can see how a strict version of `take` could be beneficial to the community, as we could encourage developers to use it unless some of the keys they are trying to take may not exist. I did a quick search in our app's codebase and found a lot of usages of `Map.take/2` that doesn't benefit from this non-strict behavior. Quite the opposite, a typo could lead to strange errors down the line that would be potentially hard to trace back.

I'm not so sure about `drop!/2`, though. The Map module doesn't contain a `delete!/2` function and I never really missed the strictness when deleting a key from a map (I guess that having some extra data is less problematic than having some missing data). Also, the current implementation of `delete/2`, used by `drop/2`, is based on a NIF, so there may be performance impacts when using the new function. But, by following the same argument of assertive code that I presented previously, if a `drop!/2` function were to be introduced in the standard library, maybe it should be worth adding a `delete!/2` function too.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/8117e4de-718f-4a8b-a6a1-374a9084c3b6%40googlegroups.com.