26

I recently received a downvote for using the following in a recent answer:

String word = ...;
if ("s".equals(word) || "y".equals(word)

The downvote was given due to using a "yoda condition". I asked for further explanation but none was provided. I prefer this style to avoid a possible NullPointerException.

Is this a poor coding style? If so, why?

Community
  • 1
  • 1
hmjd
  • 120,187
  • 20
  • 207
  • 252
  • 4
    I use yoda conditions out of preference, for the same reasons you do. – mcfinnigan Mar 27 '12 at 11:20
  • 2
    Voted to close as not constructive, since from a technical standpoint there is only a slight difference (the one you mention) for which there are reasonable alternatives (explicit `null` check) and therefore the debate invariably degenerates to the comparative merits of style, consistence, readability, etc. – Jon Mar 27 '12 at 11:21
  • 1
    Because we all know that Yoda doesn't speak good English, perhaps :-) – Edwin Dalorzo Mar 27 '12 at 11:21
  • If the question does not really fit here as suggested by the close vote, can it be migrated somewhere more appropriate? – hmjd Mar 27 '12 at 11:21
  • 7
    It happens from time to time that you get downvotes from people with weird understandings. No one can make me believe I should place the constant in the brackets. You have high enough rating (actually a lot higher than mine) to be already used to bare the bitterness of life - you can not suite everybody's mood. – Boris Strandjev Mar 27 '12 at 11:22
  • http://stackoverflow.com/questions/2349378/new-programming-jargon-you-coined/2430307#2430307 – Nishant Mar 27 '12 at 11:22
  • 1
    Jon, @hmjd asks a very clear question. If its a poor coding style and asks for a justification. Not constructive when the question wants to attempt to create a standard? – Pedro Ferreira Mar 27 '12 at 11:25
  • 3
    Good are Yoda conditions; use them I do. Like them I do. **if ((null != foo) && ( ! foo.isEmpty)** – Jim Mar 27 '12 at 11:25
  • For the record, I almost always *avoid* Yoda conditions because e.g. in this case I would prefer the explicit check: it makes it crystal clear at a glance that you won't unwittingly hit a null reference. The Yoda style is correct of course, but it's not as obviously correct. – Jon Mar 27 '12 at 11:26
  • @PedroFerreira: From the description of "not constructive": *this question will likely solicit opinion, debate, arguments, polling, or extended discussion.* – Jon Mar 27 '12 at 11:27
  • 1
    Yet it doesnt. The answer is no, it is not a poor coding style. Yoda is a choice for the reasons i've posted below in the answer. – Pedro Ferreira Mar 27 '12 at 11:52

10 Answers10

36

Bill Pugh asked this question at Devoxx 2011. The vast majority of people went for the form "xyz".equals(str). I am with Bill, now preferring str.equals("xyz").

It's fundamental to the Java tradition that we find errors as early as reasonably possible. NPEs are exceptionally common. We want to route these nulls out as soon as possible.

If you're expecting the reference to maybe null, then I don't particularly object to the backwards notation. It's nice to be explicit and easier to understand that there may be a null with a separate null check, but the reverse order should be well understood and sufficiently differentiate the code from the normal case where null is forbidden.

Working in security, some of the bugs null-tolerance accommodates are vulnerabilities.

Tom Hawtin - tackline
  • 145,806
  • 30
  • 211
  • 305
  • 1
    @hmjd also, this notation is an artifact common to other languages but unnatural to Java. We have the Single Responsibility Principle for classes http://en.wikipedia.org/wiki/Single_responsibility_principle, we have Curly's Law for variables: http://www.codinghorror.com/blog/2007/03/curlys-law-do-one-thing.html. Then having a line of code doing two things at once (avoiding nullpointers and also checking equality) seems unnatural to me in addition to being improper English and thus harder to read. Hence the fabled downvote which made you write this question. – Nikola Yovchev Mar 27 '12 at 15:02
10

Yoda conditions (i.e. putting a constant before a variable in a comparison) can be considered bad practice as it makes the line of code less comprehensible. In this specific case however I would state that using a Yoda condition makes the code more comprehensible as you don't have to put a extra null check in front of it.

Dennis Laumen
  • 3,163
  • 2
  • 21
  • 24
8

Visit the following link to understand what is meant by Yoda Conditions|Notation

Its not a "poor coding style" its diferent way of coding.

Yoda can be usefull to track typos in some languages, i believe the -1 was not deserved to be honest but that is my personal opinion.

But Yoda can be bad as explained in this lengthy but very interesting article.

End of the day, there are supporters in favor and against this kinda of notation.

Pedro Ferreira
  • 629
  • 3
  • 8
  • 1
    In the comments of the "Yoda can be bad" the author notes that the "foo".equals(bar) case is a special case where better yoda is. – valbaca Jul 01 '13 at 21:27
3

Well, it depends. If in your program "word" should never be null, word.equals("s") may actually be better. If for some obscure reason "word" will become null, you will get NullPointerException. Think about it. If you get exception, you know something went wrong, and you can faster find mistake and fix it. If program will continue to work silently, and produce wrong results, it will be much harder to detect the problem. Actually, you may not notice there is the problem at all.

It all depends.

bigGuy
  • 1,732
  • 1
  • 22
  • 37
1

There are several reasons not to do it like that, however in the end it depends on you (or the team working on your product) if you think this is bad coding style. Arguments against it are:

  • Strings are rarely null (and you shouldn't make APIs where they are because people don't expect it)
  • It feels weird to put the value you are comparing to first
  • Code style uniformity is important, because this way is the exception, you should only do it, if everyone in your team does it.

As said I don't think these arguments are very strong, nor is the reason to do it like you. So it is mostly important to just agree on one way as your coding style and stick to that.

Thirler
  • 20,239
  • 14
  • 63
  • 92
1

TL;DR; This definitely is poor coding style NOT :D

well, the yoda conditions are useful in languages where non-boolean can evaluate to a boolean value, e.g. 

int test = 0;
if ( test ){ /* do something */

but this is not allowed in Java so you don't run into problems such as forgetting '=', e.g.

if ( test = 2 ){ /* do something */

instead of test == 2

  • the compiler will not let you do this. So the yoda condition may seem unnatural to someone who has not had to care about this (because he/she didn't use any other language but Java).

This definitely is NOT poor coding style it is just not very common to see Java code using it

scibuff
  • 13,377
  • 2
  • 27
  • 30
1

yoda condition is where oup put the literal in front of the variable.

word.equals("s") is read as "word equals s"

"s".equals(word) a human reads as "s equals word"

Our brains read the first example much better and the code is clearer.

the only reason imho to use yoda conditions is to prevent assignment as in "if (42 = i)" instead of "if(42 == i)"

Sibster
  • 3,081
  • 21
  • 18
  • Note that `if (i = 42)` is also disallowed in Java, because what's between the braces must be a boolean expression and `i = 42` is not a boolean expression. – Jesper Mar 27 '12 at 12:24
1

You can write

if (word != null && (word.equals("s") || word.equals("y")))

instead of

if ("s".equals(word) || "y".equals(word))

In this case, first one will never cause any NullpointerException, but in my point of view in this case the 2nd one is better, though it is in Yoda Condition

Chandra Sekhar
  • 18,914
  • 16
  • 84
  • 125
  • [Yoda](http://en.wikipedia.org/wiki/Yoda) - it's called like that because backwards he talks. – Jesper Mar 27 '12 at 12:22
  • Since the accepted answer refers to Bill Pugh I should like to add that your *first* (and slightly longer) suggestion is exactly what he also recommended when I heard him talking (at some other occasion). It’s more explicit, it’s telling you (and the null analysis engine) that `word` may be null, which can be nice and even crucial information to have. – Ole V.V. Jun 26 '18 at 09:24
1

There is a special case pf Yoda conditional I've not seen defended, or attacked, in any of the answers, so I'll add it for reference. This is the style of:

 if(0 < x && x <= max) {

A Yoda conditional because the constant (0) is before the variable (x). The argument against Yoda conditionals is that is hinders readability. Contrast that example with the functionally equivalent

 if(x <= max && x > 0) {

Do you really think that, non-Yoda variant, is more readable? I don't.

For readability when using ordering relational operators (<, <=, >, >=), I prefer the style of these heuristics:

  • Use consistent ordering relations: > is consistent with >=, but not with < or <=; < is consistent with <=.
  • Prefer < and <= to > and >=, as the default is ascending order.
  • Place conditions that impose a lower bound on the variable before conditions that impose an upper bound, if using < and <=. Do the opposite if using > and >=.

This very often produces a Yoda conditional for the lower bound.

Raedwald
  • 46,613
  • 43
  • 151
  • 237
  • 2
    I wonder why you wrote `x <= max && x > 0` instead of `x > 0 && x <= max` which would be the more correct translation, since you want to replace the yoda condition and not shuffle the expressions. And the yoda condition only has the advantage, because "0 < x < y" is a common mathematical way to describe bounds (or "(0, y)", but that's something different :D). – Tom Sep 25 '16 at 20:54
0

One might argue that you should (unit-)test your code enough to be confident that nulls don't go where they're not supposed to. This should obviate the need for yoda conditions.

mpartel
  • 4,474
  • 1
  • 24
  • 31
  • NPE are exceptionally common. There isn't enough unit testing in the world to cover null problems. – Tom Hawtin - tackline Mar 27 '12 at 11:31
  • Yoda conditions don’t solve NPE problems. They cover them up so you don’t find them so easily (sometimes). Otherwise you are of course correct, @TomHawtin-tackline, unit testing is not going to find them all. – Ole V.V. Jun 26 '18 at 09:29