Increment the value of the map if key is already present?

Question

I have a map in which I am storing total number of bytes as the key and count as the value. I need to increment the count by 1 if the key is already present in my map. If the key is not present then start with 1.

I have the below code -

Map<Integer, Integer> userList = new HashMap<Integer, Integer>();
for (String userId : listOfUsers) {
        String sql = "select * from test_table where user_id='"+ userId + "';";
    try {
        SimpleStatement query = new SimpleStatement(sql);
        ResultSet res = session.execute(query);

        int totalBytes = 0;
        Iterator<Row> rows = res.iterator();
        while (rows.hasNext()) {
            Row r = rows.next();

            ByteBuffer client_value = r.getBytes("client_value");

            int returned = client_value.remaining();
            totalBytes = totalBytes + returned;
        }

        // does this look right?
        userList.put(totalBytes, userList.get(totalBytes) + 1);

    } catch (Exception e) {
        // log an error
    }
}

Whenever I am running for the first time, I am getting NPE on my userList.put command. Is there anything wrong I am doing? This code will be in single thread.

How about trying your code? It will tell you what is wrong with it. Oh no, it won't: catch (Exception e) { // log an error } — Jens Schauder, Feb 27 '15 at 06:43
Does it *work* right? If so then there isn't a better way. Test it, with and without the key already existing in the map. You'll then quickly identify the problem with it (hint: yes, there is a problem). — Jason C, Feb 27 '15 at 06:43
Sorry I just updated now, it is throwing NPE on userList.put command. I should have posted it after running. Sorry about that. — john, Feb 27 '15 at 06:45
@david If it throws an NPE then it isn't right. For that, check out http://stackoverflow.com/questions/218384/what-is-a-null-pointer-exception-and-how-do-i-fix-it, which will provide you with the info you need to add the logic to make sure that doesn't happen. — Jason C, Feb 27 '15 at 06:47
@david The logic implemented by the put(n, get(n)+1) is very curious, especially when one considers how this value n is composed, i.e., from a sum of bytes remaining in a ByteBuffer. There is no way to relate this value to any "solid" value in the data, such as a user id. — laune, Feb 27 '15 at 06:55
@laune looks like it is constructing a frequency table of "total client value size by user", which might be useful for statistical purposes. — user253751, Feb 27 '15 at 07:08
As a sidenote, the counting idiom you are trying to implement is done much better with a multimap: https://code.google.com/p/guava-libraries/wiki/NewCollectionTypesExplained — The111, Feb 27 '15 at 07:22
@The111 "Counting" is not the same as "storing", so multimap isn't applicable here. — laune, Feb 27 '15 at 08:11
@laune It is not clear to me what OP is accomplishing with his code conceptually (though he does use the word "count" in his first sentence). But **logically** he is mapping a key to a value which represents a count, which is precisely the situation described in the link above, and precisely something a multimap excels at, saving him the null checks and increments he is doing (possibly at the cost of more complex logic when reading from the map later, but again I'm not sure what he is ultimately using the map for). — The111, Feb 27 '15 at 08:24
@The111 The key is a length (in bytes), the value is the frequency this length occurs. What do you want to store in the Collection that is the value of a multimap? The Integer 1, many times??? — laune, Feb 27 '15 at 08:38
@laune Oops I meant multiset, sorry for the confusion and (twice) wrong term typed in my comments. — The111, Feb 27 '15 at 08:55
@The111 Yes, you can store all lengths as they occur in a multiset. Although I don't see why keeping the very many counted items (integers!) should be superior to keeping their occurrence count: Why is {1,1,1,1,1,1,2,2,2,2,2,2,2,2,3,3,3,3,3,3,3} better than {<1:6>,<2:8>,<3:7>}? — laune, Feb 27 '15 at 09:18
@laune That's an implementation detail, and most multiset implementations use a backing map rather than actually storing multiple copies of identical objects as you seem to imply. In either case you are conceptually storing {, , etc}. The reason to use the multiset interface is because it allows you to write more succint and error-free code, again as described much more thoroughly at the page I linked a few posts up. — The111, Feb 27 '15 at 09:30
Come to think of it, neither form is perfect. What you need is some class `Frequency` with `count(T item)` and assorted methods. The Map code on that page isn't best practice (any more), and obtaining the results isn't compared. — laune, Feb 27 '15 at 09:50

score 3 · Accepted Answer · edited Feb 27 '15 at 08:25

3

No, if the key is not already present you'll get a NullPointerException. That's because the expression userList.get(totalBytes) + 1 will try to unbox the null to an int.

The correct way would be to do a null check before addition.

Integer bytes = userList.get(totalBytes);
userList.put(totalBytes, bytes == null ? 1 : bytes + 1);

edited Feb 27 '15 at 08:25

The111

5,757
4
39
55

answered Feb 27 '15 at 06:45

Ravi K Thapliyal

51,095
9
76
89

I believe it would be better to encourage the OP to actually run and test his code and discover this. – Jason C Feb 27 '15 at 06:45
@david Just do a `null` check before you try to increment the value as shown in my answer. – Ravi K Thapliyal Feb 27 '15 at 06:49
@david To fix: Check the documentation of `get()`, see that it returns `null` when the element isn't found, observer that `null + 1` throws an NPE, then add logic to insure that `null + 1` never happens. You describe your desired behavior as *"I need to increment the count by 1 if the key is already present in my map. If the key is not present then start with 1."*, but you did not implement the second sentence of that description. :) Your description pretty much directly translates into the logic you need. – Jason C Feb 27 '15 at 06:49
1

@RaviThapliyal I need to find out how many users have 100 bytes, how many users have 200 bytes of data so that's why I am storing totalBytes as the key and total count as the value. – john Feb 27 '15 at 06:53

Scary Wombat · Answer 2 · 2015-02-27T06:50:35.733

1

No,

userList.put(totalBytes, userList.get(totalBytes) + 1);

The first tiem that you run this, the userList.get(totalBytes) will return a null object which of course can not be incremented.

It can be fixed as

Integer val = userList.get(totalBytes);
if (val == null) {
   val = new Integer ();
}
val = val + 1;

edited Feb 27 '15 at 06:50

answered Feb 27 '15 at 06:44

Scary Wombat

44,617
6
35
64

I believe it would be better to encourage the OP to actually run and test his code and discover this. – Jason C Feb 27 '15 at 06:46
@ScaryWombat I found out the issue when I ran it. I added the comments in my question as well. i should have waited before posting the question. – john Feb 27 '15 at 06:46

score 0 · Answer 3 · answered Feb 27 '15 at 06:55

0

You can change

userList.put(totalBytes, userList.get(totalBytes) + 1);

to

userList.put(totalBytes, userList.getOrDefault(totalBytes, 0) + 1);

if you're using Java 8, eliminating the need for manual null-checks.

answered Feb 27 '15 at 06:55

duckstep

1,148
8
17

score 0 · Answer 4 · answered Feb 27 '15 at 07:05

0

Try this below..

userList.put(totalBytes,(userList.contains(totalBytes)?(userList.get(totalBytes)+1):1));

answered Feb 27 '15 at 07:05

mady

119
1
3
12

Increment the value of the map if key is already present?

4 Answers4