I would like to refactor that code so that I do not duplicate Statements. I always need to check condition1 before any other condition. (So I cannot just change the order of the conditions). I also do not want to write &&!condition1 after every other condition.

@M.C. Refactoring conditional logic is usually something that will be different on a case-by-case basis. In this particular case, you can remove if (condition1) entirely, because it runs the exact same content as the else block. Assuming it doesn't cause side effects as jk suggests. That would be pretty 'yuck' indeed.
–
KChalouxDec 10 '13 at 13:34

2

@KChaloux this is exactly what I can not do. Executing Statement2 while condition1 applies is not correct in my context
–
M.C.Dec 10 '13 at 13:36

2

@Kieveli checking a condition does not change anything by itself here - which I agree is bad practice. The code says if (A) else if (B). Removing if (A) affects the code flow not because checking A had some side effect that's missing now. You removed the else as well. You changed the code semantically.
–
Konrad MorawskiDec 10 '13 at 15:34

As I wrote in my comment to Kieveli's answer, I see nothing wrong about multiple returns in a method, if there is no memory management considerations (as it might be the case in C or C++ where you have to release all resources manually before you leave).

Or another approach still. Here's the decision matrix so that we don't mess it up:

Which is almost exactly what Kieveli suggested, only his disdain for early returns caused his implementation to be buggy (as he noted himself), because it wouldn't do a thing if all 3 conditions were false.

Or, we could revert it like so (this probably wouldn't work in every language - it works in C#, for one, since C# allows for equality comparison between multiple variables), now we're virtually back to the first one:

// note that "condition1" on the right side of || is actually redundant and can be removed,
// because if the expression on the right side of || gets evaluated at all,
// it means that condition1 must have been false anyway:
if (condition1 || (condition1 == condition2 == condition3 == false)) // takes care of all 4 x T** plus FFF (the one special case).
{
Statement1A;
Statement1B;
return;
}
// and now it's nice and clean
if (condition2)
{
Statement2;
return; // or "else" if you prefer
}
if (condition3)
{
Statement3;
return; // if necessary
}

Quite a nice solution. I noticed that nesting makes the code overly complicated (hence the sport of bracket hunting, which I think every programmer has done at least once), and unless early return causes problems, I think this is the way to go.
–
Vlad PredaDec 10 '13 at 15:45

4

The thing I needed was not to repeat conditions nor statements. And your solution (the one starting with !condition1) is a semantically correct solution that does exactly that.
–
M.C.Dec 10 '13 at 17:53

I usually draw a K-Map, similar to the matrix though. But the K-Map suggets condition2==condition3==false(since adding columns is easier than adding a cell), which you already mentioned that you could do.
–
CruncherDec 10 '13 at 18:29

I'm not a fan of your empty if and return statements. It becomes tricky to follow, and it's generally frowned upon to have multiple return statements within the body of your code. Given your goals, I would do something like this:

Personally, I don't mind multiple return statements, but only in the case where you're validating input at the very beginning of a method.
–
KieveliDec 10 '13 at 13:40

26

it's generally frowned upon to have multiple return statements within the body of your code - I don't think it's generally frowned upon. It's frowned upon among people used to languages with manual memory management such as C (because multiple points of return make it harder to make sure that all resources are released). Personally I dislike this oldschool habit thoughtlessly transplanted into C# (or Java) code, at the cost of having to maintain a cascade of nested ifs and elses, plus a bunch of local variables, totally unnecessary otherwise
–
Konrad MorawskiDec 10 '13 at 14:42

10

You know.... I'm not sure my answer is correct. What about the case where !condition1 and !(condition2 or condition3). That should have a statement1a and statement1b as well.
–
KieveliDec 10 '13 at 14:50

2

Yes, if all 3 are false then no Statement will be executed at all.
–
Konrad MorawskiDec 10 '13 at 15:14

1

I upvoted this before @Kieveli reconsidered things and discovered the answer was wrong. (Shame on me for not looking deeper!) I'm keeping my upvote because "this answer is useful": it shows how slippery even the simple stuff can be if you're not paying attention! Keep your wits about you, people!
–
paulDec 11 '13 at 17:55

Of course, this raises the follow-up question of "when should I use a method instead of just repeating myself?", which has considerations all of its own. (I prefer "whenever possible", but also flagrantly violate that rule depending on the language.)

Maybe it's just me, but I find a condition like if (!a && b) relatively difficult to read, and generally prefer if (b && !a), when possible (i.e., unless I'm depending on short-circuit evaluation so b will only be evaluated if a was false).

The asker does care about the order of evaluation for the conditions. As mentioned by asker: "I always need to check condition1 before any other condition." So this solution cannot be used.
–
ADTCSep 10 '14 at 5:27

@ADTC: I think you're reading more into his wording than was really intended. If you read through the comments, you'll see that his concern isn't really with the order in which the conditions are evaluated, but with ensuring that statement2 is evaluated only if condition1 is false. My last sentence does cover the other possibility, but his comments made it sound as if that wasn't really the intent.
–
Jerry CoffinSep 10 '14 at 5:50

Often, if there are blocks of statements that go together, it is possible to give the function a meaningful name, increasing even more the readability of the code. But if you don't have a meaningful name, you can still make a private function with a less descriptive name (as I did in my example).

The reason why I think this is the best solution because it retains the order of conditionals, while it does achieve that you have only one copy of Statement1A and Statement1B to maintain, and it does so without introducing any nesting constructs or temporary variables. This reduces the amount of stuff you need to keep mentally track of when reading this code.

Now, there are languages (most notably C) where early returns can't always be used, but in any other mainstream language I can think of you can.

Two approaches which have not yet been mentioned are to rewrite the statements as expressions (if they are amenable to such) or (dare I say it) using a little word that starts with g. There are cases where I would consider each of those the best alternative.

Rather nasty, and not generally something I would use, but it may be reasonable in cases where condition2 and action2 are both encapsulated in a method that attempts an operation and indicates whether it succeeds and likewise condition2 and action3. In that case, it could become:

Do the default actions if either I couldn't try anything, or everything that I tried, failed.

Note that simply copying out the actions or factoring them to a different method would generally be preferable to the above style of logic or a g___. but if code in "default actions here" would need to manipulate local variables, and its exact behavior might change, encapsulating the code into an outside method may be difficult and duplicating the code would pose a risk of the duplicate versions might be changed so as to no longer match perfectly. Further, the notion that a certain control-flow statement is harmful centers around the fact that most real-world control-flow problems can be mapped nicely into common language constructs, and that when a real-world problem maps well to a language construct one should use it. If the particular program flow required by an application requires identical behavior under multiple combinations of conditions, I would posit that it's better to explicitly force execution to a common point in all such conditions than fudge around the issue.

though not always applicable it can make the code easier to read and maintain. Basically capture the different condition in an enum and create a function to analyse the context to produce the correct enum value, then switch it and run the appropriate code.

you will achieve two things :

seperate the logic code from the execution. This isolation will make it easier to fix bugs in each parts.

easy to read control flow. The switch structure is pretty intuitive, especially so if you carefully name the enum values.

Functions

Capture each block of code in a function and in the beginiing of the function put some code to decide if you should execute or not.

Here you do not get the seperation mentioned above but you do break the IF-ELSE code flow stcructure in more maneagable bits. Plus should a bug occur it will be faster to fix as it will be isolated to a single block of code.

Biggest advantage though is that a change in any one of these functions will not affect other parts of the code.

Functions do have an edge over the switch case is that they could be added dynamically. One natural extention would be something akin to the command pattern where a first part of the code analyse the context and queues actions in a list then a second part simply execute these actions sequentially.

Regardless of chosen strategy KEEP IT SIMPLE sometimes a series of if-else is enough and should just remain as-is. As a rule of thumbs if I return in the same code to fix what seems like the same problem or if I find myself often having to scratch my head to add yet a noew condition then perhaps it`s time to seek alternatives such as ones mentionned above.