Geek asks and answers

DRY if statements

I have a C ++ program where in many different .cpp files I am doing something like this:

if (!thing1.empty() && !thing2.empty())
{
    if (thing1.property < thing2.property)
        return func1();
    else if (thing2.property < thing1.property)
        return func2();
    else
        return func3();
}
else if (!thing1.empty())
{
    return func1();
}
else if (!thing2.empty())
{
    return func2();
}
else
{
   return func4();
}

I try to make func in one way, if thing1 is bigger than thing2, or vice versa, if it's the other way around, but if that doesn't exist, then I make func only for that half. Then, if it does not exist, I am doing something completely different. Properties, functions, and return types differ each time you use this template. Is there a better design for what I want to do than this ugly mess of nested-if statements?

EDIT: My sample code implemented is simplistic. Here is some of my real code, which I hope will explain the problem better (although this is much merciless):

if (!diamondsOnly.empty() && !clubsOnly.empty())
{
    if (diamondsOnly.size() < clubsOnly.size())
    {
        if (passHighCards(player.hand, getHighCards(Card::DIAMONDS), result))
            return result;
        if (passHighCards(player.hand, getHighCards(Card::CLUBS), result))
            return result;
    }
    else if (clubsOnly.size() < diamondsOnly.size())
    {
        if (passHighCards(player.hand, getHighCards(Card::CLUBS), result))
            return result;
        if (passHighCards(player.hand, getHighCards(Card::DIAMONDS), result))
            return result;
    }
    else
    {
        if (diamondsOnly.back().value > clubsOnly.back().value)
        {
            if (passHighCards(player.hand, getHighCards(Card::DIAMONDS), result))
                return result;
            if (passHighCards(player.hand, getHighCards(Card::CLUBS), result))
                return result;
        }
        else
        {
            if (passHighCards(player.hand, getHighCards(Card::CLUBS), result))
                return result;
            if (passHighCards(player.hand, getHighCards(Card::DIAMONDS), result))
                return result;
        }
    }
}
else if (!diamondsOnly.empty())
{
    if (passHighCards(player.hand, getHighCards(Card::DIAMONDS), result))
        return result;
}
else if (!clubsOnly.empty())
{
    if (passHighCards(player.hand, getHighCards(Card::CLUBS), result))
        return result;
}
+3
source share
4

, Do

, , , , , . , , .

// Decide what to do.
std::vector<Card::Suit> passOrder;
if (!diamondsOnly.empty() && !clubsOnly.empty()) {
    // .. complicated logic that adds suits to passOrder ..
}

// Do it.
for (auto suit : passOrder) {  // This is C++11 style -- alter as needed
    if (passHighCards(player.hand, getHighCards(suit), result))
        return result;
}

( , , , .)

. , , . . , passCards, , , . , passOrder.

.

:

  • Sentinels: , , , . , - , . . , , .

  • : , , , , , , , .

  • Temporaries: , , , . , /- , , , SENTINEL_VALUE, 0 -1.

:

// For readability.
const bool fewerClubs = clubsOnly.size() < diamondsOnly.size();
const bool sameNumber = clubsOnly.size() == diamondsOnly.size();
const int lastDiamondValue =  diamondsOnly.empty() ? -1 : diamondsOnly.back().value;
const int lastClubValue    =  clubsOnly   .empty() ? -1 : clubsOnly   .back().value;

// Decide what order to select cards for passing.
std::vector<Card::Suit> passOrder;
passOrder.push_back(Cards::DIAMONDS);  // default order
passOrder.push_back(Cards::CLUBS);

// Do we need to change the order?
if (fewerClubs || (sameNumber && lastClubValue > lastDiamondValue)) {
    // Yep, so start with the clubs instead.
    passOrder[0] = Cards::CLUBS;
    passOrder[1] = Cards::DIAMONDS;
}

// Do it.
for (auto suit : passOrder) {  // This is C++11 style -- alter as needed
    if (passHighCards(player.hand, getHighCards(suit), result))
        return result;
}

, getHighCards .

+5

, , :

if (thing1.empty() && thing2.empty())
   return func4();
else if (thing1.empty())
    return func2();
else if (thing2.empty())
    return func1();
else if (thing1.property < thing2.property)
    return func1();
else if (thing2.property < thing1.property)
    return func2();
else
    return func3();

; , , - . ; () . ; , .

, , return, else .

, , .

( ) " " :

  • [...]
  • [t] [...]

, , .

CardType pass[2] = { -1, -1 };  // Card::INVALID would be nice

if (clubsOnly.empty() && diamondsOnly.empty())
{
    ...do undocumented action for no diamonds or clubs...
}
else if (diamondsOnly.empty())
{
    pass[0] = Card::CLUBS;
}
else if (clubsOnly.empty())
{
    pass[0] = Card::DIAMONDS;
}
else if (diamondsOnly.size() < clubsOnly.size())
{
    pass[0] = Card::DIAMONDS;
    pass[1] = Card::CLUBS;
}
else if (clubsOnly.size() < diamondsOnly.size())
{
    pass[0] = Card::CLUBS;
    pass[1] = Card::DIAMONDS;
}
else if (diamondsOnly.back().value > clubsOnly.back().value)
{
    pass[0] = Card::DIAMONDS;
    pass[1] = Card::CLUBS;
}
else
{
    pass[0] = Card::CLUBS;
    pass[1] = Card::DIAMONDS;
}

, , , .

for (int i = 0; i < 2; i++)
{
    if (pass[i] != -1 && passHighCards(player.hand, getHighCards(pass[i]), result))
        return result;
}

...undocumented what happens here...

2 ; .

, , ( , , , ). , , , .

+4

- , , , , . "" .

// return decision1 or decision2, depending on the result of comparison of properties
// Note: type is ssize_t to accommodate bool, size_t and whatever type is 'value'
int decision(ssize_t property1, ssize_t property2, int decision1, int decision2)
{
    if (property1 > property2)
        return decision1;
    else if (property2 > property1)
        return decision2;
    else
        return 0;
}

some_func()
{
    int decision = decide(!diamondsOnly.empty(), !clubsOnly.empty(), 1, 2);
    if (!decision)
        decision = decide(diamondsOnly.size(), clubsOnly.size(), 3, 4);
    if (!decision)
        decision = decide(diamondsOnly.back().value, clubsOnly.back().value, 3, 4);

    bool flag;
    switch (decision)
    {
        case 1:
            flag = passHighCards(player.hand, getHighCards(Card::DIAMONDS), result);
            break;
        case 2:
            flag = passHighCards(player.hand, getHighCards(Card::CLUBS), result);
            break;
        case 3:
            flag = passHighCards(player.hand, getHighCards(Card::DIAMONDS), result);
            flag = passHighCards(player.hand, getHighCards(Card::CLUBS), result);
            break;
        case 4:
            flag = passHighCards(player.hand, getHighCards(Card::CLUBS), result);
            flag = passHighCards(player.hand, getHighCards(Card::DIAMONDS), result);
            break;
        default:
            flag = whatever();
            break;
    }

    if (flag)
        return result;
}

switch DRY; , , "" , :

  • 0: -
  • 1:
  • 2:
  • 3:
if ((decision & 1) == 0) {flag = whatever;}
else
{
    thing1 = (decision & 2) ? Card::DIAMONDS : Card::CLUBS
    flag = passHighCards(player.hand, thing1, result);
    if (decision & 4)
    {
        thing2 = (decision & 8) ? Card::DIAMONDS : Card::CLUBS;
        flag = passHighCards(player.hand, thing2, result);
    }
}

, "" , switch.

+3

, , IMyCompare. , IMyCompare Func

- AThing.MyCompare(otherThing);

- , .

I will be tempted by simply returning an int from MyCompare and passing on how to use it for something else that I think.

+1
source

More articles:


All Articles