Casey Crookston
Casey Crookston

Reputation: 13945

Refactor a multi-layered if-then

Looking for someone to proofread my logic. I inherited a method that contains this:

If (a || b)
{
   doTaskOne();
}
else
{
    if (c)
    {
        doTaskOne()
    }
    doTaskTwo()
}

Could this be simplified like this?

If ((a || b) || c))
{
   doTaskOne();
}
else
{
    doTaskTwo()
}

Upvotes: 3

Views: 120

Answers (4)

Keiwan
Keiwan

Reputation: 8251

These are not the same since in the first version if (a || b) is false and c == true, then both doTaskOne() and doTaskTwo() would be called, whereas your alternative only ever calls one of the two.

Upvotes: 10

ispiro
ispiro

Reputation: 27653

if (a || b || c) doTaskOne();
if (!a && !b) doTaskTwo();

Is this what you're looking for?

Upvotes: 2

SledgeHammer
SledgeHammer

Reputation: 7680

You can't really simplify that logic in any cleaner way.

Upvotes: 1

Mark Sowul
Mark Sowul

Reputation: 10600

As mentioned, no, it can't. But consider an approach along these lines:

bool runMethodOne = false;
bool runMethodTwo = false;

if (a || b)
{
   runMethodOne = true;
}
else
{
    if (c)
    {
        runMethodOne = true;
    }
    runMethodTwo = true;
}

if(runMethodOne)
  doTaskOne();
if(runMethodTwo)
  doTaskTwo();

This gives you some flexibility into tweaking the boolean logic, for example

bool runMethodOne = (a || b);
bool runMethodTwo = !runMethodOne;
if(c)
  runMethodOne = true;

Or

bool runMethodOne = false;
bool runMethodTwo = false;

if(c)
  runMethodOne = true;

if(a||b)
  runMethodOne = true;
else
  runMethodTwo = true;

(This tweak may or may not be clearer depending on the actual semantics of your code; I think the original nesting is clearer)

Upvotes: 0

Related Questions