views:

168

answers:

4

As of java 1.5, you can pretty much interchange Integer with int in many situations.

However, I found a potential defect in my code that surprised me a bit.

The following code:

Integer cdiCt = ...;
Integer cdsCt = ...;
...
if (cdiCt != null && cdsCt != null && cdiCt != cdsCt)
    mismatch = true;

appeared to be incorrectly setting mismatch when the values were equal, although I can't determine under what circumstances. I set a breakpoint in eclipse and saw that the Integer values were both 137, and I inspected the boolean expression and it said it was false, but when I stepped over it, it was setting mismatch to true.

Changing the conditional to:

if (cdiCt != null && cdsCt != null && !cdiCt.equals(cdsCt))

fixed the problem.

Can anyone shed some light on why this happened? So far, I have only seen the behavior on my localhost on my own PC. In this particular case, the code successfully made it past about 20 comparisons, but failed on 2. The problem was consistently reproducible.

If it is a prevalent problem, it should be causing errors on our other environments (dev and test), but so far, no one has reported the problem after hundreds of tests executing this code snippet.

Is it still not legitimate to use = to compare two Integer values?

In addition to all the fine answers below, the following stackoverflow link has quite a bit of additional information. It actually would have answered my original question, but because I didn't mention autoboxing in my question, it didn't show up in the selected suggestions:

Why can't the compiler/JVM just make autoboxing “just work”?

+1  A: 

The issue is that your two Integer objects are just that, objects. They do not match because you are comparing your two object references, not the values within. Obviously .equals is overridden to provide a value comparison as opposed to an object reference comparison.

MattC
Good answer, but it doesn't explain why it's only failing for 137.
Jeremy Goodell
Doh, I missed that part.
MattC
+1  A: 

Integer refers to the reference, that is, when comparing references you're comparing if they point to the same object, not value. Hence, the issue you're seeing. The reason it works so well with plain int types is that it unboxes the value contained by the Integer.

May I add that if you're doing what you're doing, why have the if statement to begin with?

mismatch = ( cdiCt != null && cdsCt != null && !cdiCt.equals( cdsCt ) );
wheaties
Good point, but I'm not doing what I'm doing. It was simplified.
Jeremy Goodell
+8  A: 

The JVM is caching Integer values. == only works for numbers between -128 and 127 http://www.owasp.org/index.php/Java_gotchas#Immutable_Objects_.2F_Wrapper_Class_Caching

Adam
Thanks, that certainly explains why 137 fails! And it also answers my question about why it's not a prevalent problem, in 95% of the cases I'm going to encounter, the value would be under 127. Good to catch this now though for the 5% where it isn't.
Jeremy Goodell
Interesting side note: up until a couple weeks ago, cdiCt and cdsCt were both ints so this was fine, but I had to make them Integers in order to check for the null situation which is handled differently ...
Jeremy Goodell
@Jeremy Yeah, it's a pretty obscure problem, but as a general rule you use .equals() for Objects and == for primitives. You can't rely on autounboxing for equality testing.
Adam
Lol, check mark back to you then! Looks like Colin has more than enough points already anyway.
Jeremy Goodell
+4  A: 

You can't compare two integer with a simple == they're objects so most of the time references won't be the same.

There is a trick, with Integers between -128 and 127 references will be the same as autoboxing uses Integer.valueOf() which caches small integers.

If the value p being boxed is true, false, a byte, a char in the range \u0000 to \u007f, or an int or short number between -128 and 127, then let r1 and r2 be the results of any two boxing conversions of p. It is always the case that r1 == r2.


Resources :

On the same topic :

Colin Hebert
Thanks, I marked the first answer as the right one though.
Jeremy Goodell
oh, I guess I can mark them both as right. Check mark for you too.
Jeremy Goodell
@Jeremy, actually you can only mark one as right. Clicking the check mark a second time only changes the accepted answer. Not to be petty; Colin's answer is excellent. :)
Adam