Skip to content

Add a Map#merge function #2644

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

Closed
DartBot opened this issue Apr 18, 2012 · 9 comments
Closed

Add a Map#merge function #2644

DartBot opened this issue Apr 18, 2012 · 9 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.

Comments

@DartBot
Copy link

DartBot commented Apr 18, 2012

This issue was originally filed by @MarkBennett


What steps will reproduce the problem?

  1. Create a map called A. (Such as one of request parameter defaults.)
  2. Use Map#forEach() to merge values from another map, B, into A.
  3. Get annoyed when doing this many times.

What is the expected output? What do you see instead?

The Map interface should define a merge function which merges together two Maps.

What version of the product are you using? On what operating system?

Version 0.1.0.201204121423, Build 6479
Dart SDK version 6478, Dartium version

@madsager
Copy link
Contributor

Added Area-Library, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Apr 19, 2012

This comment was originally written by @seaneagan


I think both mutative and non-mutative versions would be useful:

// mutative
void putAll(Map<K, V> other);
// non-mutative
Map<K, V> merge(Map<K, V> other);

However, an Object#clone would make non-mutative versions of APIs slightly less necessary:

var merged = map.clone().putAll(otherMap);

Though I'm not sure whether cloning an Map/List/etc. should produce another immutable object in which case this wouldn't work.

@DartBot
Copy link
Author

DartBot commented Apr 19, 2012

This comment was originally written by @seaneagan


Nevermind about Object#clone, looks like someone with much more experience has discoverered that copy constructors are superior:

http://www.artima.com/intv/bloch13.html

So my example would instead be:

var merged = new Map.from(map).putAll(otherMap);

which isn't so bad.

@lrhn
Copy link
Member

lrhn commented Nov 4, 2012

Issue #6415 has been merged into this issue.


cc @nex3.

@justinfagnani
Copy link
Contributor

FYI: HashMap and LinkedHashMap both have an addAll() method, but Map does not. The analyzer just starting reporting warnings for objects typed Map, and it looked like to a lot of people that Map had just removed addAll()


Set owner to @floitschG.
Added C1 label.

@lrhn
Copy link
Member

lrhn commented May 31, 2013

I must admit I thought Map had an addAll method, which is why I implemented it in HashMap.

If we add addAll, we should also consider having addMissing, where addAll uses the argument map's value if both have the key, and addMissing uses the original value.

@DartBot
Copy link
Author

DartBot commented May 31, 2013

This comment was originally written by @seaneagan


instead of:

map.addMissing(otherMap);

in many cases one could mutate otherMap:

otherMap.addAll(map);

or create a new Map:

var merged = new Map.from(otherMap)..addAll(map);

@floitschG
Copy link
Contributor

I agree. No need to implement addMissing. If we really need it, we can add it later.

I don't like the addAll name, since All usually stands for Iterables, but it's too late to change this now.

Ok to add addAll to the interface, but at the same time we need to adapt SplayTreeMap (which doesn't have addAll yet.


Added Ready-to-implement, Accepted labels.

@lrhn
Copy link
Member

lrhn commented Jun 10, 2013

Added Fixed label.

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Jun 10, 2013
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.
Projects
None yet
Development

No branches or pull requests

5 participants