12

I have a Java method with a void type of return that checks a condition. It do something in case of true and other thing in case of false, typical if / else structure, but is possible use only the if block with a empty return in the end and omit the else block placing its contents with one less level of indentation. I don't know the implications of use one or other form, is one of they a bad practice? or use one or the other is a personal election?

// The first way
private void test(String str) {
    if (str.equals("xxx")) {
        // Do something
    } else {
        // Do other thing
    }
}

// The second way
private void test(String str) {
    if (str.equals("xxx")) {
        // Do something
        return;
    }
    // Do other thing
}
Orici
  • 237
  • 1
  • 2
  • 6

2 Answers2

18

It depends on the semantics of your code, if the else branch is the point of the method, i.e. it's named after what happens in the else branch, the early return is probably correct, especially if the first part is some sort of check.

If on the other hand the two branches are both related to the methods functionality, the if/else makes more sense to use.

ex.

void frob(...)
{
   if(isBlob())
      return; //can't frob a blob
   doFrobbing(...)
}

vs.

void condOp(...)
{
  if(condition)
      conditionalthing();
  else
      otherthing();
}

making the code match the semantics of your method makes it easier to read.

esoterik
  • 3,927
3

If "something" and "other thing" are simple, straightforward, and understandable, it doesn't matter because both work.

// The first way is completely readable
private void test(String str) {
    if (str.equals("xxx")) {
        System.out.println("value is xxx");
    } else {
        System.out.println(str);
    }
    return;
}

// The second way is also readable
private void test(String str) {
    if (str.equals("xxx")) {
        System.out.println("value is xxx");
        return;
    }
    System.out.println(str);
    return;
}

However, if "something" or "other thing" are complex and hard to understand, it doesn't matter because both fail, and the solution is to refactor it into something simple enough that the code is back to the first case.

// The first way is completely readable
private void test(String str) {
    if (str.equals("xxx")) {
        doSomething(); // obfuscates the complex code you had before
    } else {
        doOtherThing(); // obfuscates the complex code you had before
    }
    return;
}

// The second way is also readable
private void test(String str) {
    if (str.equals("xxx")) {
        doSomething(); // obfuscates the complex code you had before
        return;
    }
    doOtherThing(); / /obfuscates the complex code you had before
    return;
}
/* 
 * Obviously, real code would have better variable and function names,
 * allowing the reader to understand what is happening.
 * If I care about how we doSomething(), I can investigate that function,
 * but I know that test(String) in some cases does something
 * and in other cases does the other thing, which may be enough
 */
Chris G
  • 204