11
Map<String,Integer> m;
m = new TreeMap<String,Integer>();

Is it good practice to add the following cast just to avoid the null pointer exception when m.get() is null.

System.out.println( ((Integer) 8).equals( m.get("null") ) ); // returns false

Alternatively with a prior null check it starts to look a bit ugly.

System.out.println( m.contains("null") != null && m.get("null").equals( 8 ) );

Is there a better way to write this? Thanks.

Andrew G
  • 2,596
  • 2
  • 19
  • 26

6 Answers6

9

The == operator doesn't compare values, but references.

You should use the .equals() method, instead, applied to the Integer variable (for which you are sure that is not null and NPE won't be thrown):

Integer eight = 8; //autoboxed
System.out.println(eight.equals(m.get("null")));

This will print false even the m.get("null") returns null.

Konstantin Yovkov
  • 62,134
  • 8
  • 100
  • 147
9

I try to avoid casts whenever possible, so I'd rather use the following, which also looks nicer in my opinion:

Integer.valueOf(8).equals(m.get("null"))
5gon12eder
  • 24,280
  • 5
  • 45
  • 92
  • 1
    Of course they end up generating essentially the same code. The "cast" is really an auto-boxing directive. – Hot Licks Oct 09 '14 at 00:16
  • @HotLicks It is, and casting a primitive to its respective boxing type is even statically type checked. I still don't like it for stylistic reasons. The more severe problem with the OP's code is of course using `==` to compare `Integer`s for equality as other answers have pointed out. I must admit that I have overlooked that at a first glance, answering only his immediate question whether the cast “is considered poor style” (question edited since then). – 5gon12eder Oct 09 '14 at 01:36
  • Yeah, I guess he was originally using `==`, but edited that away pretty quickly. The odd thing is that it would actually work, for small value positive ints, since they are essentially "interned". – Hot Licks Oct 09 '14 at 01:38
8

No, because it will not work. You can't compare two Integer with ==, as that compares references and not the integer values. See more info in this question

You'll need a helper method, such as:

boolean safeIntegerCompare(Integer a, int b)
{
    if (a != null) 
      return a.intValue() == b;
    return false;
}
Community
  • 1
  • 1
nos
  • 223,662
  • 58
  • 417
  • 506
  • I don't believe he was intending to do that (though I'll admit that his full intent is hard to divine). – Hot Licks Oct 09 '14 at 00:17
1

If only one of the arguments may be null (as is the case when you're comparing an unknown value to a constant), use equals() like this:

Integer foo = Integer.valueOf(8); // you could write Integer foo = 8; here too but I prefer to avoid autoboxing
if (foo.equals(m.get("x"))) { //will never throw an NPE because foo is never null
   ...
}

Note that your example isn't going to work in general because comparing non-primitive values with == only returns true if they refer to the same object instance. (Which in this case might even be true for very specific reasons but most of the time it isn't.)

biziclop
  • 48,926
  • 12
  • 77
  • 104
1

To expand the accepted answer: i find myself having to check the equality of 2 Integer variables which might or might not be null.

So my solution would be:

boolean equals = Optional.ofNullable(objectA.getNullableInteger()).equals(Optional.ofNullable(objectB.getNullableInteger());

Bertie
  • 17,277
  • 45
  • 129
  • 182
  • Wouldn't it be easier to just use `Objects.equals` here? `boolean equals = Objects.equals(objectA.getNullableInteger(), objectB.getNullableInteger())`; – Kariem Jul 15 '22 at 08:09
1

You can use Objects.equals

int foo = 1;
Integer bar = null;
Objects.equals(foo, bar);
Mohammed Aslam
  • 995
  • 9
  • 14