Nested if statements with 3 different parameters

by

I have a block of code below. The allDone() method at the bottom should only be run if the allCompleted == true. It should run through each of the statements to test.

  • allCompleted: This starts as true so the below logic works right.

  • run*.Checked: This is based on a check box in a form. This block should only run if this box is checked.

  • cmd: This is a generic string variable stating whether another part of the code (not shown here) was run successfully. If it has run successfully this string will read “done”.

After those options, if all enabled (run*.Checked == true) methods have returned the cmd* string as "done" (everything that’s checked has run successfully) then allCompleted should be true at the end so allDone() gets run.

If one single enabled method returns false (there was an error somewhere or otherwise it did not return "done"), then the allDone() method should not be run and the code will continue, skipping the last if (allCompleted) statement.

bool allCompleted = true;

if (runPart1.Checked)
    if (cmdPart1 == "done")
        allCompleted = ((allCompleted)? true : false);
    else
        allCompleted = false;

if (runPart2.Checked)
    if (cmdPart2 == "done")
        allCompleted = ((allCompleted) ? true : false);
    else
        allCompleted = false;

if (runPart3.Checked)
    if (cmdPart3 == "done")
        allCompleted = ((allCompleted) ? true : false);
    else
        allCompleted = false;

if (runPart4.Checked)
    if (cmdPart4 == "done")
        allCompleted = ((allCompleted) ? true : false);
    else
        allCompleted = false;


if (allCompleted)
    allDone();

So if at anytime one of the enabled parts fail the code will basically just move on.

As it stands this code works, I just feel like it could be written better. Is this the best way or have I got it? Something about it makes me feel awkward still.

EDIT: Also, each time one of the parts completes, it runs this method, so it will run a few times being false in the end until the last one runs and all the others are “done” in which case it should completes and run allDone().

Answer

Okay, here is how I would reduce the code duplication (if I am understanding the conditions correctly):

Edit: Original:

bool runCompleted(bool checked, string done)
{
    if( ( checked && done == "done" ) || !checked )
        return true;
    else
        return false;
}

New version based on Jerry’s feedback:

bool runCompleted(bool checked, string done)
{
    return !checked || done == "done";
}

Then in your code:

if(    runCompleted(runPart1.Checked, cmdPart1 )
    && runCompleted(runpart2.Checked, cmdPart2 )
    && runCompleted(runpart3.Checked, cmdPart3 )
    && runCompleted(runpart4.Checked, cmdPart4 )
  )
    allDone();

Attribution
Source : Link , Question Author : YourRedHerring , Answer Author : jphofmann

Leave a Comment