4

我有一个 C++ 程序，在许多不同的 .cpp 文件中，我执行以下操作：

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

如果thing1大于thing2，我会尝试以一种方式执行func，或者如果相反，我会尝试以一种方式执行func，但是如果不存在，那么我只为那一半执行func。如果两者都不存在，我会做一些完全不同的事情。每次使用此模式时，属性、函数和返回类型都不同。对于我想做的事情，有没有比这种丑陋的嵌套 if 语句更好的设计？

编辑：意识到我的示例代码过于简单化了。这是我的一些真实代码，希望能更好地解释问题（尽管它更混乱）：

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;
}
4

4 回答 4

5

决定然后做

查看真实代码，我注意到的第一件事是有许多几乎相同的调用，它们仅相差一个常数。我会使用在复杂逻辑中设置的参数在一个地方进行调用。

// 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。

简化逻辑

接下来我们要简化决策逻辑。

选项：

  • 哨兵：部分复杂性来自这样一个事实，在某些情况下，您需要取消引用其中一个容器中的最后一张卡，如果容器是空的，您就无法这样做。有时值得考虑在容器中添加哨兵，这样您就不需要测试空箱——您可以保证它永远不会是空的。这可能可行，也可能不可行。您需要使处理容器的所有其他代码都理解哨兵。

  • 只是例外：您可以通过选择默认顺序来消除某些子句，例如，先是钻石然后是梅花，然后只测试您需要先是梅花然后是钻石的情况。

  • Express with 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 处理可能为空的容器作为输入。

于 2012-05-10T17:26:16.120 回答
4

我不确定这是否是一个巨大的改进，但你可以通过以下方式消除一些麻烦：

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每次都可以删除。

给定一个更大的例子，那么你的所有代码都会导致一个或两个非常相似的动作，这取决于某些情况。

在这种情况下，应该应用 Kernighan 和 Plauger的优秀（但略微过时且已绝版）书“编程风格的要素”中的一句格言：

  • 子例程调用允许我们总结参数列表中的不规则性 [...]
  • [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 有点不舒服；它出现两次。

但是，总的来说，这为您提供了一个线性序列的测试，每个测试之后都有简单的对称动作（为了一致性，这次保留了大括号，因为动作不止一行；一致性比大括号本身的存在与否更重要）。当关于做什么的决定完成后，你就真的去做了。

于 2012-05-10T14:35:32.510 回答
3

您可以计算所有条件并将它们发送到某个函数，该函数应该做出决定，并返回一个告诉下一步该做什么的代码。所有重复的“模式”都将在函数内部。

// 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 点：首先要做什么
  • Bit 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);
    }
}

然而，在我看来，这个“符合 DRY 标准”的作品看起来比switch.

于 2012-05-10T16:49:16.123 回答
1

创建一个接口说IMyCompare。使用一个接受一个 IMyCompare 并返回一个 Func 的方法然后你可以在每件事上实现整个事情

并做一些类似 AThing.MyCompare(otherThing);

如果每个条件的哪个函数不是由事物的类型确定的，则将一个函数数组传递给比较调用。

我很想从 MyCompare 返回一个 int，并将如何使用它委托给我认为的其他东西。

于 2012-05-10T14:46:07.213 回答