Boxed value unboxed then reboxed

Question

FindBugs is giving me a warning about the following line, where invoiceNumber is an Integer object:

text.append(String.format("%010d-", (invoiceNumber == null) ? 0 : invoiceNumber));

The warning is: "Boxed value is unboxed and then immediately reboxed"

Now I think I understand (un)boxing, but I can't see how you would do the same thing without getting the warning?

I have found that I can get rid of the warning using the following code instead, but this seems more long-winded:

int invNo = (invoiceNumber == null) ? 0 : invoiceNumber;
text.append(String.format("%010d-", invNo));

Can someone show me what is the 'correct' way to do the above?

BTW, I've looked at the related questions and I understand what was going on with them, but this doesn't seem to match any of those.

Amaris

Perhaps text.append(String.format("%010d-", (invoiceNumber == null) ? Ineger.valueOf(0) : invoiceNumber)); will do. – Fenestella

Yes, that does get rid of the warning, but why is that 'better'. It's constructing an Integer object (which should be expensive), but it shouldn't need to. – Amaris

In the case of 0 (or any integer from -128 to 127), it's not constructing any new Integer instance, since the instance is already available in the IntegerCache. And you are saving the unboxing and boxing operations, so it might give you a slight performance improvement. – Fenestella

Actually you do construct a new Integer in your original code as well as in your fix with int invNo = (invoiceNumber == null) ? 0 : invoiceNumber; (though FindBugs warning disappears), but using @Fenestella solution you will not construct the new Integer, but reuse the existing one. Well though Eran answer is correct (upvoted) in general you may just ignore this warning. It's very unlikely that it will actually harm the performance of your application. The created extra Integer is very short-lived. – Omega

Answer 1

The type of the (invoiceNumber == null) ? 0 : invoiceNumber) expression is int. It requires unboxing of invoiceNumber in the case invoiceNumber is not null.

On the other hand, String.format expects a single String argument followed by Object references, which means your int get immediately boxed to Integer again.

You can try to avoid the original unboxing by using (invoiceNumber == null) ? Integer.valueOf(0) : invoiceNumber), which would make this expression return an Integer.

Fenestella

So does that mean that my 2 line 'solution' in my question is actually less efficient, because the invoiceNumber is unboxed to invNo, and then reboxed to an Integer for the String.format? – Amaris

@DuncanKinnear, exactly! – Omega

@Amaris I'm assuming that your two line 'solution' only prevents FindBugs from detecting the unboxing and reboxing, so it doesn't actually change anything compared to your original code. – Fenestella

@Amaris No, the second version has the same inefficiency (save the assignment to a one-time-use local variable which the compiler probably optimizes away). – Archibold

Answer 2

Try changing (invoiceNumber == null) ? 0 : invoiceNumber to (invoiceNumber == null) ? 0 : invoiceNumber.intValue(). I think the warning comes from you using the Integer again in the "false" case, not an int.

Unbraid

Nope, same warning. I tried casting the false value to (int) and I still got the warning. – Amaris

The warning when away for me…what version of Java are you using? – Unbraid

Are you sure you are using the exact same code, including the String.format? – Amaris

Yes. What is the type of text? A StringBuilder? StringBuffer? – Unbraid

StringBuilder. Probably not worth pursuing as Eran's solution seems a better fit. His explanation would also explain why the intValue() doesn't fix the 'problem' as the resulting int is still being boxed for use as the String.format argument. – Amaris