Proposal: Add Map.take!/2

[Sabiwara Yukichi]

Hi,

Would it make sense introducing a trailing bang version mirroring the existing Map.take/2? It would behave the same but fail with a KeyError on a missing key instead of simply ignoring it.

To give some context, I've lately found myself writing things like `%{foo: data.foo, bar: data.bar, ...}` quite frequently, a typical use case being to dump only some fields from a map or struct into a JSON payload or Elasticsearch. While it's tempting to refactor those using `Map.take/2`, it feels best to fail frankly than omitting fields accidentally in order to avoid potential bugs.

I suppose we would also introduce Keyword.take!/2 for consistency if we move forward.

What do you think?

Cheers,

Sabiwara

[José Valim]

Sounds good to me, but we will also need Map.drop! for consistency?

[Sabiwara Yukichi]

Good point. So, to sum up: Map.take!, Keyword.take!, Map.drop! and Keyword.drop! ?

I'll open a PR :)

Le lun. 25 oct. 2021 à 22:13, José Valim <jose....@dashbit.co> a écrit :

Sounds good to me, but we will also need Map.drop! for consistency?

--
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/CAGnRm4KSt%2BhuMUJue7aaaipQCFOEZzeM-_XNnUUOwQTErU5kFA%40mail.gmail.com.

[José Valim]

One thing to note though is that pattern matching is going to be the most efficient in most cases, if you want the actual values. So the docs need to be clearer that this is to return a new map with said keys.

[Sabiwara Yukichi]

I see your point. Map.take/2 might be misused as well, should this note be added to Map.take or Map.take! (or both)?

Additional question: if we are adding drop!/2, we should probably use delete!/2 as well (note: pop!/2 already exists), what do you think?

--

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/CAGnRm4J7%2Brq8nTq7sU2XqAZ55gSnVFJq2LBey32tpMAVwx0eNw%40mail.gmail.com.

[José Valim]

It doesn't quite apply to `Map.take/2` because the field may not be there anyway, so you can't easily pattern match, you always need a case.

I also see why we would add "delete!" but at the same time, "delete!" does not have much purpose. The result won't have said field anyway.

Now I am thinking it may be better to not add `Map.take!/2`. It is not complicated to implement it yourself and given the possible confusion with pattern matching and that it may require both drop! and delete!, it is probably not worth it. So for now I would like to postpone adding this functionality. Sorry.

[Sabiwara Yukichi]

> It doesn't quite apply to `Map.take/2` because the field may not be there anyway, so you can't easily pattern match, you always need a case.

Right, but people might be using it on maps when knowing the keys are present (e.g. a struct) as well?

> I also see why we would add "delete!" but at the same time, "delete!" does not have much purpose. The result won't have said field anyway.

Indeed, I cannot think of any actual use case for this one. But maybe drop! would be a bit similar?

Maybe we could consider only introduce take! in this case, given the fact we already have pop! without having delete!.

> So for now I would like to postpone adding this functionality. Sorry.

No problem, I understand the concerns and implications :) Thank you for the discussion.

--
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/CAGnRm4LZsHUBFkBH19w4-UNgApYuBAQj4K4vrhAu43NQz47%3D5g%40mail.gmail.com.

[Caio Câmara]

https://github.com/whatyouhide/short_maps

I think we can get some inspiration from this.

--
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/CAGnRm4LZsHUBFkBH19w4-UNgApYuBAQj4K4vrhAu43NQz47%3D5g%40mail.gmail.com.

[José Valim]

> Maybe we could consider only introduce take! in this case, given the fact we already have pop! without having delete!.

In my mind, pop! is fetch!+delete, so I think it is fine to not have delete!. But take/drop were always closely related.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAK5L3ppWMPOx53p%2Bxp8%3DZwsJnFrWysVaSBxxAH%2BeXnMZzFX%3DDg%40mail.gmail.com.