Which is better?


public void foo()
{
    if ( someCondition()  || !otherCondition() ) {
        if(!otherCondition() ){
            //do other stuff
        }
        return 1;
    } 

    else {
        return 2;
    }
}

Ew, checking the same condition twice? I’d never do that.
Especially as you’ve no idea how expensive evaluating ‘otherCondition()’ is. (It’s returned value might even change, or have side-effects!)

(p.s. has anyone noticed the examples have been returning an int for a void method? :slight_smile:

What do people think of this paradigm:


public int foo()
{
   final int retVal;

   // all the shiz that has been outlined in previous examples....
   // ....but assigning to the final int instead of immediately returning.

   return retVal;
}

public void foo()
{

    boolean b = otherCondition()

    if ( someCondition()  || !b) {
        if(!b){
            //do other stuff
        }
        return 1;
    } 

    else {
        return 2;
    }
}

Edit: forgot to change it, lol.

This thread is so funny…

People are trying to ‘optimize’ a code sample that was created for discussion about indentation, because two branches return the same value, probably by accident.

Nothing to see here, move along people.

Yeah someone posted that above too.

I worked with someone who I really respected who did that so I started as well, but in the end I decided it wasn’t worth it as sometimes it reduces readability (which is more important in my opinion).

The whole idea of kicking out of a method if you have something “invalid” is much clearer than a giant if statement around the actual body of the code; “if my object isn’t null and I am allowed to be here right now then” is much more confusing than “if my object is nill, return, if I can’t be here now, return” in my opinion.

The idea of having only one return statement is to reduce potential code paths you can do down and therefore reduce bug count, but I’ve honestly never found a bug related to return values if you keep things readable and small. A random return inside the middle of a massive method, on the other hand, is a bad idea because it’s not immediately obvious how one gets there.

I was in particular pointing out the use of a final; forcing the code to assign the return value only once, which IMO has a positive knock-on effect for code structure & complexity.

I don’t think there is necessarily a ‘right’, or even a ‘better’ answer to this question; it very much depends on the circumstances of the method. (size, complexity, control structures).
I even sometimes find labeled breaks to be more readable than the equivalent for/while loop! (as they are self-documenting, often reduce the size of the loop condition, and keep the logic & it’s effect together.

However there are definitely lots of wrong answers =)

Just in case you were wondering: I wasn’t addressing you with my ‘rant’.

Anyway, I feel that much of this is part of your own coding style and your own preferences. I too prefer to keep the indentation to a minimum. Early return is comparable to throwing an exception: it should be done to abort the expected flow as soon as possible. Throwing an exception in the middle of nowhere is just as bad as returning at some ‘random’ location.

It’s all about readability, because if it’s readable it means it ‘maps to your mind’, which means it’s easier to comprehend, analyze and debug. I think it shows your mind also aborts incorrect thought patterns as soon as possible, as opposed to building a stack of thoughts. Obviously I base this on absolutely nothing.

Whoever wrote it is right.

And if you want it done the right way you do it yourself.

This solves so many dilemmas far beyond computer programming!

Cas :slight_smile:

I like both, depending on the situation. The second one saves a few bytes though :wink:

None of them, I prefer this:

private Object checkSomething() {
  final Object result;
  if(getSomeObject() == null) {
    result = something;
  } else {
    result = somethingElse;
  }
  return result;
}

because there is only one return statement and I’m forced to set the variable.

Why would you waste your fingers on typing “final” there?

Cas :slight_smile:

I would guess he’s doing it so that you’re very clear that variable can only be assigned once. However, I’d argue that his code is perhaps more confusing - I always as a rule assign constants on the line they are defined, because anything else looks wrong to me.

I make all my variables final unless they need to be none final. Knowing that a variable will never change makes decoding my code later much easier because I don’t have to worry about tracking it. Now a non-final variable is clearly going to be altered within an algorithm. Essentially it’s more obvious which bits will change and which won’t.

I have used a few functional languages and so kinda like the trust you can have in the way that a variable will never change.

[quote=“JL235,post:33,topic:35884”]
+1. 90% of the variables and method parameters I declare are final. Even more if you ignore loop variables (enhanced for helps too). Actually, my code would be much cleaner if all variables were implicitly final and there was a “mutable” keyword instead.

final, FTL! It is just clutter on parameters and local. It isn’t harder to debug with non-final variables. Unless you need final for scoping, you’re just wasting your time (and mine when I have to read it). :slight_smile:

Indeed, it’s really not hard to see where something is being assigned. Adding final all over the place is just more clutter!

Cas :slight_smile:

If java would be invented now, I’d vote for final being the default as I declare everything final by default. Maybe it’s clutter, but it’s concise and clear.
Now that final isn’t default, I think it would help if Eclipse’s (or any IDE’s) syntax colouring would distinguish between final and non-final variables (I’d colour non-final variables orange, probably :)).

[quote]I’d vote for final being the default as I declare everything final by default.
[/quote]
Agreed.
I set everything as final. I find it helps a little to prevent bugs.

It prevents no bugs at all and makes your code hideous. There’s only one place final should be used, and that’s for security related purposes to prevent overriding critical methods or classes. Which means for us ordinary developers it has no purpose at all. Except constants :wink:

Cas :slight_smile:

And closures. Otherwise totally agreed.