Increment array value using array[key]++

Question

My Java code:

        if(wins.containsKey(winner)) {
            int currentCount = wins.get(winner);
            wins.remove(winner);

            wins.put(winner, currentCount + 1);
        } else {
            wins.put(winner, 1);
        }

This was my alternative to something I can do in PHP and even C#:

if(isset($something[$key])) {
    $something[$key]++;
} else {
    $something[$key] = 1;
}

This is going to be used in a high number of iterations in a for loop so I would like to consider performance. Is this whole remove() then puts() business killing the performance? What is an alternative?

Well, for one thing, wins isn't an array (as the title states). With arrays in Java, you can happily use []. — Bernhard Barker
– Bernhard Barker, Commented Sep 30, 2013 at 19:02

Rohit Jain · Accepted Answer · 2013-09-30 19:00:32Z

5

Your code can be replaced to:

if(wins.containsKey(winner)) {
    wins.put(winner, wins.get(winner) + 1);
} else {
    wins.put(winner, 1);
}

No need to remove the entry. When you add another entry in the map with same key, it will overwrite the existing one.

answered Sep 30, 2013 at 19:00

Rohit Jain

214k45 gold badges419 silver badges534 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Jon Skeet Over a year ago

It's still performing a containment check, a get, a put and a boxing operation for every iteration. While it's almost certainly not a problem, it's not as fast as it could be.

Rohit Jain Over a year ago

@JonSkeet. Hmm. The only real issue is unboxing and boxing. Rest we can't avoid IMO. Not a real performance gain over the original code though.

Jon Skeet Over a year ago

You can definitely avoid doing a containment check followed by a get (see the second part of my answer) and you can avoid doing a put other than for "new" entries (see the first part).

Alcanzar · Accepted Answer · 2013-09-30 19:05:24Z

1

The way I usually do this to be as efficient as possible is:

Integer val = wins.get(winner);
wins.put(winner,val == null ? 1 : (val + 1));

This is very clean to me and avoids the extra hash lookup to "get" the val after you know it's already in there from the contains.

answered Sep 30, 2013 at 19:05

Alcanzar

17.2k7 gold badges45 silver badges60 bronze badges

Comments

Jon Skeet · Accepted Answer · 2013-09-30 19:07:34Z

1

Firstly, I strongly suspect that this isn't going to be a performance bottleneck. As ever, test the simplest code that works before you use more complicated code.

You could use AtomicInteger instead of Integer as the value type of your map. That would allow you to mutate the wrapped value, rather than replacing the whole entry. Then you'd have:

if(wins.containsKey(winner)) {
    wins.get(winner).incrementAndGet();
} else {
    wins.put(winner, new AtomicInteger(1));
}

If you can stick with Integer, you could still optimize your code further:

Integer previousValue = wins.get(winner);
int newValue = previousValue == null ? 1 : (int) previousValue + 1;
wins.put(winner, newValue);

Now there is exactly one get and one put operation on each iteration.

edited Sep 30, 2013 at 19:07

answered Sep 30, 2013 at 19:01

Jon Skeet

1.5m893 gold badges9.3k silver badges9.3k bronze badges

1 Comment

Rohit Jain Over a year ago

Liked the 2nd version. Didn't think of that way. :)

Collectives™ on Stack Overflow

Increment array value using array[key]++

3 Answers 3

3 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related