ConcurrentHashMap foreach loop problem

Question

I have a concurrenthashmap called users. I have user objects in it with some integer keys that is not id. I want to find the user with a given id. Therefore, I check all elements of hashmap and return the user object if it is present. Here is my code :

    for(User u : users.values()) {
        logger.error("u.getId() : " + u.getId());
        logger.error("id : " + id );
        if( u.getId() == id ) {
            logger.error("match");
            return u;
        }
    }
    logger.error("Not found: id:" + id);
    for(User u : users.values()) {
        logger.error(u.getPos() + ". user: " + u.getId());
    }

However even tough my u.getId() and id are the same I cannot see "match" at my logs.

213 matches but It can not enter the following if statement. here are my logs:

enter image description here

What do you think about it?

Eugene Kuleshov · Accepted Answer · 2011-09-27T15:24:30.337

6

What type returned from User.getId() method and what type of id variable? If it is not a primitive type, you need to use equals() instead of ==.

By the way, a good static code analyzer like FindBugs can find such kind of errors.

edited Sep 27 '11 at 15:24

answered Sep 26 '11 at 19:09

Eugene Kuleshov

31,461
5
66
67

you are right. they are Long. I should have used equals. and it explains comparing them true sometimes. I'll try now. – aykut Sep 26 '11 at 19:12

score 4 · Answer 2 · answered Sep 26 '11 at 19:09

You haven't shown the types involved, but is it possible that id or getId() is an Integer instead of an int? If so, you'll be comparing references, so you should just use

if (u.getId().equals(id))

to compare the values within the Integer objects. Be careful if getId can return null though...

ConcurrentHashMap foreach loop problem

2 Answers2