Combine if blocks with repetitive code

Question

I have this piece of code

if (!expr1) {
    codeblock1;
} elseif (expr2) {
    codeblock2;

    codeblock1;
}

It is pissing the hell out of me because I am trying to refactor it in such a way that there is no repetitive code, but I keep getting different results.

if (!expr1 || expr2) {
    if (expr2) {
        codeblock2;
    }

    codeblock1;
}

What is the logical difference between these two examples?

Answer 1

In snippet 1 you are explicitly stating that you don't want codeblock2 to be run if expr1 is false.

In snippet 2 you are stating that you DO want codeblock2 to be run, even if expr1 is false, as long as expr2 is true.

Also, assuming that codeblock1 and codeblock2 are a series of lines of code (more that just one line), you can extract them to methods. That way you could do:

if (!expr1) {
    codeblock1(); // calling a method
} elseif (expr2) {
    codeblock2(); // calling a method
    codeblock1(); // calling a method
}

... which is not cosidered code repetition.

Answer 2

In this specific case, a nice way to refactor your code would be to make the conditions clearer. (Make sure to put the following code into a function)

if (expr1 and !expr2)
    return;
if (expr1 and expr2)
    codeblock2();
codeblock1();

If you read the code line by line, you see:

• If expr1 is true, and expr2 is false, don't do anything, leave the function
• If expr1 is true and expr2 is true, run codeblock2
• run codeblock1

It makes it very clear what conditions you are actually checking for, and what happens when that specific condition is met.