DRY if语句 [英] DRY if statements

问题描述

我有一个C ++程序，在许多不同的.cpp文件中，我这样做：

I have a C++ program where in many different .cpp files, I do the 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();
}

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

I'm trying to do func one way if thing1 is bigger than thing2, or backwards if the opposite is the case, but if one doesn't exist then I only do func for that half. Then if neither exist, I do something completely different. The properties, functions, and return types are different each time I use this pattern. Is there a better design for what I want to do than this ugly mess of nested-if statement?

编辑：实现我的示例代码是过度简化。这里有一些我的真实代码，希望能更好地解释问题（虽然它更麻烦）：

Realized my example code is an oversimplification. Here's a bit of my real code that hopefully will explain the problem better (although it is much messier):

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

推荐答案

决定然后做

看一下真实的代码，我注意到的第一件事就是有很多几乎相同的调用只有一个常数变化。我会使用在复杂逻辑中设置的参数在一个地方进行调用。

Looking at the real code, the first thing I notice is that there are a lot of nearly identical calls that vary only by a constant. I would make the calls in one place using a parameter that's set in the complex logic.

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

（如果向量总是只有一两个，那么使用向量可能会过度，但是我假设真正的代码可以处理所有套装。）

(Using a vector may be overkill if it's always just one or two, but I'm assuming the real code might deal with all the suits.)

这使得它更容易阅读。程序员可以看到，首先你决定通过卡的顺序，然后你实际上是通过它们。两个单独的步骤将更加清晰。只有一个调用passCards的地方使得它不容易出现愚蠢的拼写错误而不是让它的副本分散在整个决策逻辑中。它也会使调试变得更容易，因为你可以在非常特殊的情况下设置断点，或者你可以在循环开始时设置一个断点并检查passOrder。

This makes it easier to read. The programmer can see that first you're deciding the order to pass cards and then you're actually passing them. Two separate steps are going to be clearer. Having just one place that calls passCards makes it less prone to stupid typos than having copies of it scattered throughout the decision logic. It's also going to make it easier to debug, as you can set breakpoints on very specific cases, or you can just set a breakpoint at the beginning of the loop and inspect passOrder.

简化逻辑

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

Next we want to simplify the decision logic.

选项：

• Sentinels：复杂的部分原因在于，在某些情况下，您需要取消引用最后一个卡在其中一个容器中，如果容器是空的，则无法执行此操作。有时候考虑将一个哨兵添加到一个容器中是值得的，这样你就不需要测试空盒子了 - 你可以保证它永远不会是空的。这可能是可行的，也可能是不可行的。你需要让处理容器的所有其他代码都了解哨兵。

• Sentinels: Part of the complication comes from the fact that, in some cases, you need to dereference the last card in one of the containers, which you cannot do if the container is empty. Sometimes it's worth considering adding a sentinel to a container so that you don't need to test for the empty case--you'd be guaranteed that it's never empty. This may or may not be workable. You'd need to make all the other code that deals with the containers understand the sentinel.

只是例外：你可以通过选择来消除一些条款默认订单，例如钻石，然后是球杆，然后只测试你需要球杆然后是钻石的情况。

Just the Exceptions: You could eliminate some of the clauses by choosing a default order, e.g., diamonds then clubs, and then test only for the cases where you'd need clubs then diamonds.

快递与临时工：创造良好 - 命名的临时工具，简化您必须进行的比较，并根据这些临时表达比​​较。请注意，如果将空/非空案例分解为临时案例，则可以通过选择适当的SENTINEL_VALUE（例如0或-1）来消除某些情况。

Express with Temporaries: Create well-named temporaries that simplify the comparisons you have to make and express the comparison in terms of these temporaries. Note that with the empty/not-empty case factored out into the temporary, you can eliminate some of the cases by choosing an appropriate SENTINEL_VALUE, like 0 or -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处理可能为空的容器作为输入。

This assumes that getHighCards copes with a possibly empty container as input.

这篇关于DRY if语句的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！