7

I came across a code snippet which iterates over a map using its entry set and performs some action only if entry != null

As far as I know even if we don't enter anything in map map.entrySet returns an empty set and not null. Even if I put {null,null} then the entry will be [null=null] i.e an instance with these elements. But the instance won't be null.

Map<String, String> map = new HashMap<String, String>();
        map.put(null, null);
        map.put(string1, string1);
        for(Map.Entry<String, String> entry : map.entrySet()){
            if(entry != null){
                                  //do something
            }

        }

I have below basic questions:

  1. Under what scenario an entry in HashMap will be NULL?
  2. Is the check even valid

I strongly believe if(entry != null) over caution and it should be removed.I just want to be sure.

Raedwald
  • 46,613
  • 43
  • 151
  • 237
Abhinav
  • 1,720
  • 4
  • 21
  • 33
  • The check may be there to just check whether the object is initialized to tackle NullPointerException. See this answer http://stackoverflow.com/questions/14283684/null-check-for-hashmap-key – Kishor Pawar May 18 '15 at 07:07

4 Answers4

7

An iterator could return nulls for collections that support null values, but as you yourself showed this isn't possible for Maps. The check is redundant and misleading.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • 1
    Thanks..and just to add to your answer as I digged in myself,got the below implementation from "HashMap.java" `public Set> entrySet() { return entrySet0(); } private Set> entrySet0() { Set> es = entrySet; return es != null ? es : (entrySet = new EntrySet()); }` It will never return a NULL value. – Abhinav May 18 '15 at 07:29
4

The scenario is invalid. This is code from the hashmap implementation

private Set<Map.Entry<K,V>> entrySet0() {
    Set<Map.Entry<K,V>> es = entrySet;
    return es != null ? es : (entrySet = new EntrySet());
}

So, you should not get a null value

bobK
  • 704
  • 2
  • 7
  • 24
0

The check is obviously redundant. @Kayaman's answer is nice.
But, I don't agree with your comment and @bobK's answer.

I think you may mix up entry and entrySet. The check has nothing to do with the imp of method entrySet().The method entrySet() just ensure the entrySet is not null, while the judge in your example is to ensure that the entry in the Set is not null.

Set is able to contain a null object, therefore sometimes we need to do the NPE protection. The reason why we don't need to do the check here is that the Map Class ensure the entry in entry set is not null. The method forEach() in EntrySet class is the important one.

    public final void forEach(Consumer<? super Map.Entry<K,V>> action) {
        Node<K,V>[] tab;
        if (action == null)
            throw new NullPointerException();
        if (size > 0 && (tab = table) != null) {
            int mc = modCount;
            for (int i = 0; (i < tab.length && modCount == mc); ++i) {
                for (Node<K,V> e = tab[i]; e != null; e = e.next)
                    //e is not null
                    action.accept(e);
            }
            if (modCount != mc)
                throw new ConcurrentModificationException();
        }
    }

e != nullis ensured here.

-1

The check may be there to just check whether the Map is instantiated to avoid NullPointerException in further references to the object.
If you think this is for caution you better make sure the Map has been instantiated in your constructor

Kishor Pawar
  • 3,386
  • 3
  • 28
  • 61