Refactor if-else control flow

Viewed 139

I came across the following snippet while going through the code:

if(a || b){
    if(a) {
        doSomething();
    }
    doSomethingElse();
} else {
    throw new Exception("blah");
}

I was wondering how can I refactor this code for better readability (or it's already in optimal shape?). Below is my first attempt:

if(!a && !b){
    throw new Exception("blah");
}
if(a){
    doSomething();
}
doSomethingElse();

Does this look better?

5 Answers

Only condition a || b execute doSomethingElse:

if(a) {
    doSomething();
} 
if (a || b) {
    doSomethingElse();
} else {
    throw new Exception("blah");
}

When negating condition as !a && !b it can get confusing, I prefer positive conditions

You can also do it as

if(a) {
    doSomething();
    doSomethingElse();
} else if(b) {
    doSomethingElse();
} else {
    throw new Exception("blah");
}

Just go through the logical process. You can check for a, then doSomething() and doSomethingElse(), else if b, then doSomethingElse(), else Exception:

if (a) {
    doSomething();
    doSomethingElse();
} else if (b) {
    doSomethingElse();
} else {
    throw new Exception("blah");
}

You can do two separate checks instead:

if(a){
  doSomething();
}

else if(a || b){
  doSomethingElse();
}

else throw new Exception("Exception");

This format is well readable to me personally :)

In my opinion, a code becomes more readable if instead of comparing the values in your if statement, you compare them and store the result in a variable with a helpful name that gives you some information about the context.

Let's say that being "a" or "b" is being a "valid" value. Then it would look like that:

boolean isValid = a||b;
if(isValid){
    if(a) {
        doSomething();
    }
    doSomethingElse();
} else {
    throw new Exception("blah");
}

That way you know that you're checking the condition of "being valid".

Related