Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why Map#merge() limited to joining 2 maps? #21

Closed
bdistin opened this issue Jul 15, 2018 · 2 comments
Closed

Why Map#merge() limited to joining 2 maps? #21

bdistin opened this issue Jul 15, 2018 · 2 comments

Comments

@bdistin
Copy link

bdistin commented Jul 15, 2018

It seems when doing joins, currently, the most common behavior is being able to join/merge a virtually unlimited number of other structures.

Examples:

  1. Array#push(value1[, value2[, ...[, valueN]]]);
  2. Array#unshift(value1[, value2[, ...[, valueN]]]);
  3. Array#concat(array1[, array2[, ...[, arrayN]]]);
  4. Object.assign(object1[, object2[, ...[, objectN]]]);

Exceptions:

  1. Set#add();

Which you are happening to be fixing by adding Set#addAll(). (Side note: In my opinion, it would be better to change the semantics of Set#add() to allow adding several elements, than to add Set#addAll(), if possible)

Why didn't I list Map#set() as an exception? Because Map#set() seems more like an assignment operation, like Object.defineProperty(). Although to be fair, there is enough room for Object.defineProperties() in the lang, so it may be a mistake I separate it. I suppose that's up to interpretation.

For consistent design, I think the proposed Map#merge() should allow: Map#merge(iterator1[, iterator2[, ...[, iteratorN]]]);. What do you think about this?

@zloirock
Copy link
Contributor

Side note: In my opinion, it would be better to change the semantics of Set#add() to allow adding several elements

It will break code like array.forEach(set.add, set) so it's unacceptable.

I think the proposed Map#merge() should allow: Map#merge(iterator1[, iterator2[, ...[, iteratorN]]])

Makes sense.

@bdistin
Copy link
Author

bdistin commented Jul 15, 2018

Ah, I see what you mean with Set#add(). I will strike that note from the OP.

@Ginden Ginden closed this as completed in 7da035b Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants