programming with short-circuit evaluation in Java

Question

Does relying on short-circuit evaluation make the code fragile? I wrote a piece of code that essentially looks like the following. My professor wanted me to rewrite it.

(Note: I know for sure that only one of the four conditions will be true, because given any stream, there is only one 'next token', right?)

foo getFoo()
{
        Bar bar;
        if ((bar = peekAndGet('x')) != null 
                || (bar = peekAndGet('y')) != null 
                || (bar = peekAndGet('z')) != null 
                || (bar = peekAndGet('t')) != null) 
            return produce(bar);
        else 
            return null;
}

Is this really fragile? I find it working perfectly. But how should I rewrite it?

Answer 1

I would refactor it and write it like this:

char[] values = {'x', 'y', 'z', 't'};
for (char c : values) {
    Bar bar = peekAndGet(c);
    if (bar != null) return produce(bar);
}
return null;

Note: one good reason to do it, is that the first time I read your code I thought it looked buggy until I read your question. You want to keep those "Something looks wrong" moments for things that really are wrong.