Is there a more readable way of making a long chain of && statements?

Question

Suppose I have a long, complex list of conditions, which must be true in order for an if statement to run.

if(this == that && foo != bar && foo != that && pins != needles && apples != oranges)
{
    DoSomethingInteresting();
}

Typically, if I'm forced into doing something like this, I'll just put each statement on its own line, like this:

if
(
         this == that 
    &&    foo != bar 
    &&    foo != that 
    &&   pins != needles 
    && apples != oranges
)
{
    DoSomethingInteresting();
}

But I still feel this is a bit of a mess. I'm tempted to refactor the contents of the if statement into its own property like this

if(canDoSomethingInteresting)
{
    DoSomethingInteresting();
}

But then that just moves all the mess into canDoSomethingInteresting() and doesn't really fix the problem.

As I said, my goto solution is the middle one, because it doesn't obfuscate the logic like the last one and is more readable than the first. But there must be a better way!

Example in response to Sylon's comment

bool canDoSomethingInteresting
{
    get{
        //If these were real values, we could be more descriptive ;)
        bool thisIsThat = this == that;
        bool fooIsntBar = foo != bar;
        bool fooIsntThat = foo != that;
        return 
        (
               thisIsThat
            && fooIsntBar
            && fooIsntThat
        );
    }
}
if(canDoSomethingInteresting)
{
    DoSomethingInteresting();
}

Is each of your conditions logically distinct, or do some belong together? Would it make sense to combine subsets of your conditions, for example? — Jon Skeet, Jan 08 '13 at 05:32
I wouldn't consider the third solution obfuscation. If the method name reflects what it does then IMO it makes the important code more readable. In situations where I cannot avoid conditional logic like that I use something like the 3rd solution containing conditions arranged like the 2nd. (Makes it easy to tinker & tune) — Steve Py, Jan 08 '13 at 05:34
It could be either I suppose. I'm not solving one specific problem here, but looking for more of a pattern or better way of thinking. Let's go worst case scenario and pretend they're all logically distinct, because that entails greater complexity. — Iain Fraser, Jan 08 '13 at 05:36
I'll often introduce an explaining variable (and put one condition per line in the variable initializer), so that the line breaks at least aren't inside the parentheses just before the indented block. — Joe White, Jan 08 '13 at 05:36
@StevePy would you simply write your method or property using the same layout as the second if statement and substitute `if(..)` for `return(...)` or would you do something else? — Iain Fraser, Jan 08 '13 at 05:43
Substituted for `return` typically. Depending on the usage this could be a method within the class, or something like a Validator class which makes code nice and test-able. — Steve Py, Jan 08 '13 at 23:25

score 6 · Accepted Answer · answered Jan 08 '13 at 05:32

6

In my opinion moving the mess into a Property or a Method is not that bad an idea. That way it is self contained and your main logic where you do the if(..) check becomes more readable. Especially if the list of conditions to check is huge, it is better off being in a property, that way if you need to re-use that you are not duplicating that check.

if(IsAllowed)
{
   DoSomethingInteresting();
}

answered Jan 08 '13 at 05:32

lahsrah

9,013
5
37
67

2

Also, then it becomes self documenting, the method name tells (or should tell) what it does. – hyde Jan 08 '13 at 05:35
I must admit, I've done this sometimes and I'm quite happy with how the code ends up looking. However, what would the property end up looking like? That's my problem with this approach, it makes the immediate code more readable, but just sweeps the hard-to-read code under the carpet. – Iain Fraser Jan 08 '13 at 05:39
Well if you have a huge list of if checks and you want to make it simpler then maybe you need to look into some other form of refactoring - can your domain objects be reorganized to make things simpler? or an alternative way this can be done in some declarative AOP way with the use of attributes marking the various properties with certain rules that then your IsValid method checks using reflection. This will work well for simple validation e.g. [Required]public string Name; – lahsrah Jan 08 '13 at 05:43
Well, a way I have done it before is to make descriptively named bool for each statement and then use those in the return value of the property/method. Haha, but I also wonder sometimes if I'm labouring the point when I do it. (will write example above) – Iain Fraser Jan 08 '13 at 05:47
1

The question is also quite open ended and the actual problem, the rest of your code, refactorability of the logic etc all would determine the best approach. In some cases with stupid business logic this may be unavoidable. – lahsrah Jan 08 '13 at 05:49
Also, excellent point on the ability to re-use. This alone is enough to sway me toward refactor-into-property for these types of problems. – Iain Fraser Jan 08 '13 at 06:01

score 1 · Answer 2 · answered Jan 08 '13 at 06:05

Containing the code for the different conditions in separate vars is a great way to go for readability and maintainability. When your vars are named good you get

if (goToNextPage) 
{
    if (notAdmin)
    {
        RedirectToNormalPage();
    }
    else
    {
        RedirectToAdminPage();
    }
}

Compared to something like this

if ((x == 1) && ((y == 'r') || (y == 't'))) 
{
    if (!a)
    {
        RedirectToNormalPage();
    }
    else
    {
        RedirectToAdminPage();
    }
}

I will let you choose which one you would want to read when you go back to your code at a later date.

score 0 · Answer 3 · answered Jan 08 '13 at 05:46

Method is good. Better method name would tell what it tests, not what can be done if it is true. Like

if (isFooValid()) { mergeFoo(); }

If it fits logically to the code flow and especially if I don't care if it is done or not at that point, I also often wrap whole if to a method:

maybeMergeFoo();

The key is to mentally step back and look what fits best to the current code, and to the code you are going to write next. Often it is then clear what is the right way to organize the code.

Is there a more readable way of making a long chain of && statements?

3 Answers3