2

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
}
  • 5
    Possible duplicate of Should I return from a function early or use an if statement? – amon Jul 16 '18 at 22:24
  • It is related to that duplicate but not identical. I'd recommend using the approach that reads most naturally with the specific context it is used, rather than make a fixed rule. – Martin Spamer Jul 16 '18 at 23:23
  • Either is fine, just don't use both together!! – Erik Eidt Jul 17 '18 at 1:29
  • Possible duplicate of Clarification of "avoid if-else" advice – gnat Jul 17 '18 at 1:40
  • 2
    The question is tagged "cyclomatic-complexity", but no issue about cyclomatic complexity is actually raised in the question. However, if this is something you're concerned about, note that cyclomatic complexity is calculated from the number of branching points in the code, and that doesn't change between if return and if else. Both of your example methods have a CC of 2. – Jules Jul 17 '18 at 2:06
4

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.

  • I think, your example don't respond exactly to my question, the return in the first code example is a simple "safeguard clause", the if block only check if a condition is valid before to continue the execution of code, it don't execute nothing in it. – Orici Jul 17 '18 at 7:16
  • @Orici: Imagine if the "safeguard clause" (as you call it) would also log a message that the safeguard clause was activated. While it is technically not just a return statement, it really still is a "safeguard clause". The question is where you draw the line of what is a safeguard clause and what is not. Finding a universal rule that is pedantically correct, in my opinion, is a futile effort. In most cases, the surrounding context of the method already makes it abundantly clear whether it's a "safeguard clause" or not. – Flater Jul 17 '18 at 12:37
  • @Orici I meant if your code resemebles the safeguard clause use that pattern, if it doesn't use what ever is cleaner – esoterik Jul 17 '18 at 17:53
  • @esoterik I really like making it about the name. – candied_orange Jul 20 '18 at 3:17
0

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
 */
  • 2
    The last return is completely unuseful in a void method. – Orici Jul 17 '18 at 7:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.