39

I was asked this question in interview. Which of the following is better to use

 MyInput.equals("Something");

Or

"Something".equals(MyInput);

Thanks

jmj
  • 237,923
  • 42
  • 401
  • 438
SuperMan
  • 3,532
  • 12
  • 45
  • 49
  • 19
    Although the second solution is robust against `MyInput` being null (as others have pointed out in the answers), I do think that the first solution is better in terms of readability (if you can be sure that MyInput is non-null). – Heinzi Apr 19 '11 at 05:53

7 Answers7

63

I would go for 

"Something".equals(MyInput);

in this case if MyInput is null then it won't throw NullPointerException

Here we are sure that the object on which equals() is going to invoke is NOT NULL.

And if you expect NullPointerException from your code to take some decision or throw/wrap it, then go for first.

There is no performance impact

jmj
  • 237,923
  • 42
  • 401
  • 438
  • 16
    If a variable is `null` and I don't expect it to be, I'd rather fail fast and be told by an exception than have it fail silently, or potentially in a much more difficult to debug manner. And the first is more readable and natural, an important part of good code. – Kevin Nov 14 '12 at 01:57
  • 7
    I don't believe such an answer has so much upvotes and is considered correct! This is bad practice written all over it - switching variable/constant values to AVOID a NullPointerException is a terrible way of inviting potentially crazy behavior to production. If you expect your input to ever be null, then check it BEFORE the String comparison. If you NEVER expect it to be null, then this needs to blow hard in your application so that you can investigate the possible user stories that lead to this null value and treat them accordingly. – SiN Jan 21 '15 at 10:22
  • we are not talking about best practice here, also it is mentioned in answer already – jmj Jan 22 '15 at 05:33
21

To be the contrarian.... :)

The first line might crash if MyInput is null, but that is just a code-convenience programmers (usually with a C hangover) use when they don't want to assert that 'MyInput' can be null.

If the second option is used, then maybe this line won't cause a NullPointerException, but the following lines might.

I believe that it is better know the possible state of your variables rather than rely on some code-construct that eases your conscience.

Ryan Fernandes
  • 8,238
  • 7
  • 36
  • 53
  • While yes, I'm still nursing my hangover ... sometimes maybe I *expect* `MyInput` to be null, and that's ok. It really does have a use ;) – Brian Roach Apr 19 '11 at 06:01
  • you're assuming that a null value is not a valid value for myInput when you assume it will cause an NPE further down the line. That's not an assumption you can make without more information. – jwenting Apr 19 '11 at 07:15
  • 5
    I used to write in the `null` carpet-sweeping manner. But then, when I started Java I used to accept `null` to represent an empty collection. NPEs are far too common. `null` is pretty meaningless in the context of code. Therefore, prefer to avoid `null`s, and that means checking. And, relevant here, don't try to sweep any errors under the carpet. – Tom Hawtin - tackline Apr 19 '11 at 10:12
  • 3
    If you know `myInput` may be `null`, then tell the reader that by writing `myInput != null && myInput.equals("Something")`. It’s longer, yes, but much more informative, so worth it. Now the reader knows `myInput` may be null (crucual knowledge if you’re to modify the code) and knows you have thought about it at taken it into account. Or if you insist it be short, `Objects.equals(myInput, "Something")`. – Ole V.V. Feb 18 '17 at 10:06
5

Well how about we write our whole code upside down for a change?

Those who like their constants first, how would they feel when they see this?

if ( 2 == i)

Hiding a NullPointerException, in my opinion, is never a benefit but a shortcoming in the design.

If you never expect a NullPointerException but got one, then you need to let your application blow, follow the logs and see why this happened. It could be a business case that you missed altogether :)

If you optionally expect a null parameter and are not interested in handling it separately, then use a utility method like StringUtils.equals(...)

That said, I never allow any of my team members to use the second form because it is not consistent and not readable.

SiN
  • 3,704
  • 2
  • 31
  • 36
3

The former will raise a NullPointerException if MyInput is null, while the latter will just return false, so the latter may be preferable in certain cases (or possibly the former, if you don't expect MyInput to be null and want to fail fast).

Daniel Lubarov
  • 7,796
  • 1
  • 37
  • 56
1

If you want to be a real smarty-pants you could point out the possibility that MyInput could be a special subclass of String that has over-ridden the equals and hashcode methods. In that case the ordering of the statement is vitally important.

Here's a real-life example - how about if you want to compare Strings that have numbers in them and you want leading zeroes ignored? For example, Lecture1 would equal Lecture01.

Legs
  • 189
  • 1
  • 5
  • 3
    You'd have to use a different method or a `Comparator`, because `String` can't be subclassed, Mr. Smartypants. – Petr Janeček Jun 06 '12 at 09:29
  • @Slanec Nothing says that `MyInput` is a String, actually. (so why it cannot be a subclass of String, it can be anything else (even a primitive, in which case the first case does not compile)). – njzk2 Jan 07 '15 at 18:17
-2

i would go to "something".equals(myInput); because variable can be null simply it throw a exception if variable is null .

p27
  • 2,217
  • 1
  • 27
  • 55
-5

A good developer will always try to avoid a NullPointerException, hence the best answer would be to use "Something".equals(myInput).

Asherah
  • 18,948
  • 5
  • 53
  • 72
user1419913
  • 1
  • 3
    So you basically answered a year old question with the same thing the accepted answer said in April 2011?! – mbinette Oct 20 '12 at 05:09
  • 6
    A "good developer" will check for null before the call or prefer the exception, rather than let it fail silently and potentially introduce bugs in a much less predictable manner. And more importantly, the first way is more readable and much more natural. – Kevin Nov 14 '12 at 02:05
  • 3
    "A good developer will always try to avoid a NullPointerException" - is that your pick up line? – Mukus Mar 01 '13 at 03:05
  • Another way to avoid NullPointerException is to have try...catch(Throwable) wrapped around that area :) – SiN Nov 21 '13 at 10:29