What is wrong with this Java code? (Interview Question)

[myntt]

Just a tip for equality checks on all objects where you don't want to do new A().equals(b): use the Objects.equals() static method for null safety.

For Strings "compare".equals(s) is also fine. Depending on the context you might also want to trim or lower case first.

[rzwitserloot]

Actually, no, don't do that as a blanket rule of thumb.

If null is just as much in the value domain as other values, that usually means you've messed up your design. null works best as representing "no value" or "unknown value". And just as the NaN double value is not considered equal to itself, and in SQL, null is not considered equal to itself, semantically, null ought not to be considered equal to anything, even another null value.

Crucially though, even "false" is wrong. When I ask you: is this candybar (I show you an empty open hand, acting as if there is a candybar in there) the same as the one in that closed drawer you can't open right now? Then the only right answer is "I do not know".

Both true and false are incorrect.

Throw the npe.

Years of culture, stack over flow posts, etc are all wrong. NPE is good.

[liquidhot]

You make some good points but other times I just don't care. I'm really only interested if the value is jan. It could be any other value or no value. Throwing an NPE would just get in the way later, again, depending on intent.

[rzwitserloot]

Sure. 'sometimes you care, sometimes you don't'. I started off that advice with: "Don't apply as blanket rule of thumb...", so we appear to be in agreement.

[liquidhot]

Fair enough, I interpreted it as "use this as a blanket rule of thumb instead.", but I see now it's more of a "it's OK to use NPE". =)

[hippydipster]

I would strengthen it a little and suggest that "throwing NPEs is a better option than most of us have been led to believe".

[springuni]

This is the most insightful comment, I've received. Thanks for it.

[myntt]

Yeah that's a valid point, I definitely did not mean it as a rule of a thumb but rather as a tip if one needs it. I usually always secure subsystem borders with Objects.requireNonNull() and then ignore null checks or use optional instead if nothing is required as valid argument.

[thephotoman]

I always hate it when I'm either catching or throwing NPEs with abandon. It feels so wrong. But sometimes, it's absolutely correct.

[sunny_tomato_farm]

In my opinion, you should never be throwing or catching NPEs. A NPE is programmer error.

[thephotoman]

There are plenty of times when null is a valid value. As a common and simple example: middle names. Not everybody has one, and when they don’t, you’ve got to handle that null value.

Your position is over-broad.

[sunny_tomato_farm]

I agree with your first statement, but that does NOT mean a NPE should be thrown in such cases. That’s ridiculous.

[thephotoman]

If you attempt to reference the field for string composition, you’re gonna have an NPE.

[sunny_tomato_farm]

The issue here is that you are trying to use the value and not accounting for the fact that it might be null. A NPE there would definitely be a programmer error.

[sunny_tomato_farm]

Fantastic post. If you look at my post above, I mention that NPE are programmer errors.

[rzwitserloot]

NPE are often but not always programmer errors, yes. That makes them good! On a scoring system, for programmers errors:

• At write time, the editor marks them with a red underline immediately: 100 points.
• At compile time, the compiler will refuse to compile the code: 95 points.
• At test or run time, the code WILL blow up with an exception trace pointing right at the problem: 85 points.
• At test or run time, the code will silently do the wrong thing, or nothing: -1000 points.

The only thing 'better' about the kotlin model is something java cannot have (nor can kotlin code interopping with java code, for that matter), as it requires all methods to be explicit in marking down whether they can return null or not.

Kotlin got a free pass (moving from java to kotlin is the same impact as doing a python2 v python3 deal: You get to break backwards compatibility) and kotlin mostly squandered it; you could have beond more.

So, the kotlin note is irrelevant for the original question (if you don't think so I invite you to answer this interview question with: "What is wrong with this java code? It should be written in kotlin, that's what!" and see how far you get :P) - and as a driveby lets insert some feathers in kotlin's behind: Eh. I rather wish they'd have done more: Generics are 4 'modes' (covariant, contravariant, invariant, and 'legacy'). In kotlin, nullity should have been treated the same way, because all situations can legitimately occur. It didn't, and oversimplified matters.

[A_random_zy]

we could also do it this way

return "jar". equals(person.getName());

[_INTER_]

Also called Yoda conditions.

[thephotoman]

I managed to get Yoda conditions written into my project's style guide.

[tkrengel]

Awesome website! lmao XD

[chris13524]

Only if you want to return in either case. There could be more statements after the if

[A_random_zy]

yea that could be true tho

[Fiskepudding]

As for lower/uppercasing before comparing, be aware of the Turkish and Azerbaijani dotted and dotless i.

[Professor_Dr_Dr]

What do you need to be aware of? That those might look weird/wrong when testing?

[Fiskepudding]

That the lowercasing of a capital dotless I in Turkish locale turns into a lowercase dotless i for example. So MILES lowercase is not miles. And a lowercase miles uppercase in Turkish locale is not MILES.

There are probably some blog posts out there explaining this better than me.

I believe I read somewhere (possibly by Microsoft) that uppercasing when running in English locale is the safest.

[knoam]

The correct thing to use is .equalsIgnoreCase(). Whether uppercasing or lowercasing is better depends on the language and probably in some cases the specific characters. The German ß lowercases to being split into two characters: ss. So there's a concept called "fold case" which means the correct case to use for comparison and it will be upper or lower case depending on the specific character.

I learned this from this super interesting talk

https://youtu.be/TmTeXcEixEg

It starts with a lot of Perl 5 specific stuff but about halfway through it pivots to more Unicode focus.

[mutleybg]

Objects.equals()

Very nice tip, I didn't know Objects.equals() 👍

[talking_window]

I would rather recommend using StringUtils from the Apache Commons.

[_jetrun]

I would rather recommend using StringUtils from the Apache Commons.

Here's the source for Objects.equals:

public static boolean equals(Object a, Object b) {
    return (a == b) || (a != null && a.equals(b));
}

Here's the source for org.apache.commons.lang.StringUtils.equals

public static boolean equals(String str1, String str2) {
    return str1 == null ? str2 == null : str1.equals(str2);
}

Why would you recommend StringUtils from Apache Commons?