Add amount of all person with same id from HashMap

Question

I have a person class

public class Person {
    private int id;
    private int amount;
    public Person(int id, int amount){
        this.id = id;
        this.amount = amount;
    }
 //getters
}

And I have a mainClass like this

Map<String, Person> mapOfObjects = new HashMap<String, Person>();
mapOfObjects.put("map1", new Person(1, 1000));
mapOfObjects.put("map2", new Person(1, 1000));
mapOfObjects.put("map3", new Person(1, 1000));
mapOfObjects.put("map4", new Person(2, 1000));
mapOfObjects.put("map5", new Person(2, 1000));
mapOfObjects.put("map6", new Person(3, 1000));
Map<Integer, Integer> mapOfSum = new HashMap<Integer, Integer>();
int sum = 0;
List list = new LinkedList(mapOfObjects.keySet());
for (int i = 0; i < mapOfObjects.size(); i++) {
    for (int j = 1; j < mapOfObjects.size() - i; j++) {
        if (mapOfObjects.get(list.get(i)).getId() == 
            mapOfObjects.get(list.get(j)).getId()) {
                sum += (mapOfObjects.get(list.get(i)).getAmount() + 
                        mapOfObjects.get(list.get(j)).getAmount());
        }
        mapOfSum.put(mapOfObjects.get(list.get(i)).getId(), sum);
    }
}
System.out.println(mapOfSum);

It gives me output:

{1=8000, 2=8000, 3=0}

but i want something like this

id=1 amount =3000, id =2 amount = 2000, id =3  amount =1000

How can i remove the object from the map whose summation is already done once while running the first for-loop

Your question is not clear. please try to explain what you are trying to do better — amit, Sep 04 '14 at 10:17
The `if` inside the loop is probably wrong, you are always comparing `mapOfObjects.get(list.get(i)).getId()` with itself. — Jongware, Sep 04 '14 at 10:18

Michał Schielmann · Accepted Answer · 2014-09-04T11:03:09.167

Your loop is overengineered.

Try this:

for (Person person : mapOfObjects.values())  //for every person in the map...
{
    int newAmount = 0;
    if (mapOfSum.containsKey(person.getId()))  //if its already in the sumMap
    {
        newAmount = mapOfSum.get(person.getId()) + person.getAmount(); //update the value
    }
    else
    {
        newAmount = person.getAmount();  //else set it as starting value
    }
    mapOfSum.put(person.getId(), newAmount); //put it in the sum map. If it's already there it will be substituted by new entry.
}

Output:

{1=3000, 2=2000, 3=1000}

EDIT

If you want the output in different format you can do something like this:

StringBuilder stringBuilder = new StringBuilder("");
for (Map.Entry<Integer, Integer> entry : mapOfSum.entrySet())
{
    stringBuilder.append("id=").append(entry.getKey()).append(" amount=").append(entry.getValue()).append(", ");
}

System.out.println(stringBuilder.toString());

Output:

id=1 amount=3000, id=2 amount=2000, id=3 amount=1000,

EDIT2:

As suggested in comments by @NickHolt there is more efficient way that shortens execution time by eliminating the containsKey() check.

It can be done like this:

Integer personAmount = mapOfSum.get(person.getId());
if (personAmount == null)
{
    personAmount = person.getAmount();
}
else
{
    personAmount = person.getAmount() + personAmount;
}
mapOfSum.put(person.getId(), personAmount);

or in shorter form using ternary operator:

Integer personAmmount = mapOfSum.get(person.getId());
personAmmount = personAmmount == null ? person.getAmount() : person.getAmount() + personAmmount;
mapOfSum.put(person.getId(), personAmount);

It's more efficient to do the `get` from `mapOfSum` into a `sum` variable first and check for `null`, than a `containsKey` followed by a `get` - something like this: `Integer sum = mapOfSum.get(persion.getId())` updating the `mapOfSum` like this: `mapOfSum.put(persion.getId(), sum != null ? sum + person.getAmount() : person.getAmount())` — Nick Holt, Sep 04 '14 at 10:44
@NickHolt I've updated my answer with your suggestion. Thanks! — Michał Schielmann, Sep 04 '14 at 11:04
@NickHolt - I thought about it one more time. Now there is Integer created in every loop pass - isn't that more expensive that cointainsValue() check? — Michał Schielmann, Sep 04 '14 at 11:06
@MichałSchielmann not sure what you mean, either way every loop creates an `Integer` when it's autoboxed to put it into the `Map` — Nick Holt, Sep 04 '14 at 15:59

score 0 · Answer 2 · edited May 23 '17 at 12:12

You can use Google Guava multimap

Multimap<Integer, Integer> PersonMap = ArrayListMultimap.create();
PersonMap.put(1, 1000);
PersonMap.put(1, 1000);
PersonMap.put(1, 1000);
PersonMap.put(2, 1000);
PersonMap.put(2, 1000);
PersonMap.put(3, 1000);

Then you can use something like:

Hashset<Integer, Integer> PersonDictionary = new Hashset<Integer, Integer>();
for (Integer key: PersonMap.keySet()) 
  PersonDictionary.Add(key, Sum(PersonMap.get(key)));

Where sum a function : Is there possibility of sum of ArrayList without looping

score 0 · Answer 3 · answered Sep 04 '14 at 10:46

Instead of using two for loops, use single, for traversing the value set of the map, and find the summation

Map<String, Person> mapOfObjects = new HashMap<String, Person>();
mapOfObjects.put("map1", new Person(1, 1000));
mapOfObjects.put("map2", new Person(1, 1000));
mapOfObjects.put("map3", new Person(1, 1000));
mapOfObjects.put("map4", new Person(2, 1000));
mapOfObjects.put("map5", new Person(2, 1000));
mapOfObjects.put("map6", new Person(3, 1000));
Map<Integer, Integer> mapOfSum = new HashMap<Integer, Integer>();
for (Person person : mapOfObjects.values()) {
    Integer value= mapOfSum.get(person.getId());
    if (value == null) {
        value = 0;
    }
    value += person.getAmount();
    mapOfSum.put(person.getId(), value);
}
System.out.println(mapOfSum);

Add amount of all person with same id from HashMap

3 Answers3