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
}
7
  • 6
    Possible duplicate of Should I return from a function early or use an if statement?
    – amon
    Commented Jul 16, 2018 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. Commented Jul 16, 2018 at 23:23
  • Either is fine, just don't use both together!!
    – Erik Eidt
    Commented Jul 17, 2018 at 1:29
  • Possible duplicate of Clarification of "avoid if-else" advice
    – gnat
    Commented Jul 17, 2018 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
    Commented Jul 17, 2018 at 2:06

2 Answers 2

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.

5
  • 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
    Commented Jul 17, 2018 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
    Commented Jul 17, 2018 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
    Commented Jul 17, 2018 at 17:53
  • @esoterik I really like making it about the name. Commented Jul 20, 2018 at 3:17
  • Instead of if (str.equals("xxx")) it is safer to use if ("xxx".equals(str)). This might spare you an NPE when the str is null Commented Jan 21, 2020 at 9:18
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
 */
2
  • 6
    The last return is completely unuseful in a void method.
    – Orici
    Commented Jul 17, 2018 at 7:13
  • 3
    Obfuscate generally has a negative connotation when it comes to code readability, I think "abstracts" or "pulls out" is the word you're wanting.
    – Ecksters
    Commented Dec 18, 2020 at 21:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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