문제

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();
}
도움이 되었습니까?

해결책

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();
}

다른 팁

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.

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.

라이센스 : CC-BY-SA ~와 함께 속성
제휴하지 않습니다 StackOverflow
scroll top