25

在我的 C# 代码中,我有一个很天真地开始的 if 语句:

if((something == -1) && (somethingelse == -1) && (etc == -1)) {
    // ...
}

它正在增长。我认为现在必须有20个条款。

应该如何处理这个?

4

18 回答 18

30

尽可能使用大门。

if 语句

if(bailIfIEqualZero != 0 && 
   !string.IsNullOrEmpty(shouldNeverBeEmpty) &&
   betterNotBeNull != null &&
   !betterNotBeNull.RunAwayIfTrue &&
   //yadda

重构版本

if(bailIfIEqualZero == 0)
  return;

if(string.IsNullOrEmpty(shouldNeverBeEmpty))
  return;

if(betterNotBeNull == null || betterNotBeNull.RunAwayIfTrue)
  return;

//yadda
于 2009-06-08T17:11:18.233 回答
18

将其分解为一个函数,并使每个条件成为一个保护子句:

int maybe_do_something(...) {
    if(something != -1)
        return 0;
    if(somethingelse != -1)
        return 0;
    if(etc != -1)
        return 0;
    do_something();
    return 1;
}
于 2009-06-08T17:06:52.467 回答
17

假设所有这些条件确实是必要的,您可以将这些条件合并到一个或多个 uber-booleans 或函数调用中以增强可读性。

例如,

bool TeamAIsGoForLaunch = BobSaysGo && BillSaysGo;
bool TeamBIsGoForLaunch = JillSaysGo && JackSaysGo;

if (TeamAIsGoForLaunch && TeamBIsGoForLaunch && TeamC.isGoForLaunch())
于 2009-06-08T17:12:23.880 回答
9

要考虑的一件事是为什么你有这么多子句。正如 SWITCH 语句通常表明您应该将选择移到子类中一样,大量复杂的 IF 语句链可能表明您在一个地方组合了太多的概念(以及因此的决策)。

作为一个例子,我将使用计算佣金的例子。在我建立的一个系统中,佣金率取决于给予批发商的佣金金额,批发商将一些佣金转给零售商。批发商给零售商的金额取决于零售商和批发商之间的具体协议(基于合同)。批发商获得的金额同样取决于产品线、特定产品和销售产品的数量。

除此之外,还有基于客户状态、特定产品定制等的额外“例外”规则。这可以在复杂的 IF 语句链中解决,但我们改为由应用程序数据驱动。

通过将所有这些信息放入表格中,我们就能够传递数据规则。首先会触发批发商的通用规则,然后会触发任何覆盖规则。然后,有了基础批发商佣金,我们将运行批发商到零售商的通用规则,然后是那些例外。

这将一个巨大的逻辑毛球变成了一个简单的四步过程,每一步都简单地进行数据库查询以找到要应用的正确规则。

这当然可能不适用于您的情况,但通常像这样的大型复合体实际上意味着没有足够的类来划分责任(我们问题的另一个解决方案可能是包含特定规则和覆盖的工厂类),或者功能应该是数据驱动的。

于 2009-06-08T17:54:02.213 回答
8

将其重构为一个函数。

bool Check()
{
  return (something == -1) && (somethingelse == -1) && (etc == -1);
}

或者,您可以在 Check 函数中构建更具可读性的代码/逻辑。

于 2009-06-08T17:06:07.490 回答
6

有很多方法可以解决这个问题,但让我选择一些。

首先,所有条件(if 语句中的所有 AND 条件)+ 如果它们都为真则要执行的代码是一次性的情况。

在这种情况下,请使用您拥有的代码。您可能想要执行其他几个人已经建议的操作,重写以使用 Guard 子句类型的代码。

换句话说,而不是这样:

if (a && b && c && d && ......)
    DoSomething();

...您重写为类似于此的内容:

if (!a) return;
if (!b) return;
if (!c) return;
if (!d) return;
if (!...) return;
DoSomething();

为什么?因为一旦你开始将 OR 标准引入到组合中,就很难阅读代码并弄清楚会发生什么。在上面的代码中,您在每个 AND 运算符 (&&) 上拆分条件,因此代码变得更易于阅读。基本上,您将代码从“如果这个和那个,或者那个其他事情和那个第三个事情,或者其他事情,然后做某事”重写为“如果这个,然后退出;如果那个其他事情;然后退出;如果一些其他事情;然后退出;如果以上都不是,做点什么”。

但是,在许多情况下,您也有可重用性的情况。如果其中一些标准出现在其他地方,但实际执行的代码(DoSomething)不一样,那么我会再次选择其他人已经提出的建议。将条件重写为Boolean根据评估条件的结果返回结果的方法。

例如,什么更容易阅读,这个?

if (a && b && c && d && e && f && (h || i) && (j || k) || l)

或这个:

if (CanAccessStream() && CanWriteToStream())

假设所有这些字母都可以分为这两个标准。

在这种情况下,我会采用一些标准并将其放入这些方法中,并为标准选择一个合适的名称。

第三个选项是代码中的几个地方的标准不同,但要执行的实际代码是相同的。

在这种情况下,我会重写,以便您将标准组合在一起并将方法分层,以便调用一个方法将检查一些标准,然后调用另一个方法,它将检查一些其他标准等。

例如,你可以这样写:

if (stream != null && buffer != null && inBuffer > 0 && stream.CanWrite)
  stream.Write(buffer, 0, inBuffer);
else
    throw new InvalidOperationException();

或者你可以这样写:

if (inBuffer > 0)
{
    Debug.Assert(buffer != null);
    WriteToStream(buffer, inBuffer);
}

...

private void WriteToStream(Byte[] buffer, Int32 count)
{
    if (stream.CanWrite)
        stream.Write(buffer, 0, count);
    else
        throw new InvalidOperationException();
}

我想说第二种方式比第一种方式更容易阅读,并且更可重用。

于 2009-06-08T18:02:58.063 回答
4

对您的值列表执行聚合操作。

if (new[] { something, somethingelse, ... }.All(x => x == -1)) {
}

*编辑:给数据一个额外的行:

var Data = new[] { something, somethingelse, ... };
if (Data.All(x => x == -1)) {
}
于 2009-06-08T17:16:46.887 回答
3

看起来您有 3 条信息共同代表应用程序中的特定状态。与其打开这 3 个状态,不如创建一个封装它们的值?然后,您可以在创建时使用对象层次结构或委托来绑定您尝试运行的操作。

于 2009-06-08T17:09:03.010 回答
3

您可以将其重构为一个函数,并返回一个 Enum 值,该值代表正确的情况:

if(something != -1)
    return MyEnum.Something;
if(somethingelse != -1)
    return MyEnum.SomethingElse;
if(etc != -1)
    return MyEnum.SomethingElseEntirely;
return MyEnum.None;
于 2009-06-08T17:09:45.117 回答
3

您还可以将状态分解为一个类。

class mystate
{
    int something;
    int somethingelse;
    int etc;

    bool abletodostuff()
    {
        return (something == -1) && (somethingelse == -1) && (etc == -1);
    }
}
于 2009-06-08T17:13:59.463 回答
1

您可能会探索的另一个途径是使用复合表达式。这些表达式(这是 LINQ 在 .NET 框架中工作的基础)允许您根据所有这些条件创建表达式树,然后业务逻辑代码可以简单地对顶级表达式进行操作以获取真/假结果.

要评估表达式,您可以使用访问者模式

这使您可以轻松地组合条件树。这些树甚至可以序列化,让您坚持做出决定的条件。这里有很多机会。

于 2009-06-08T18:19:52.640 回答
1

我不知道 C#,但它似乎包括条件运算符。如果你的条件很短,你可以用漂亮的表格结构替换长的 if/elsif/else 链,如下所示:

return   something == 0      ? 0
       : somethingelse == -1 ? 1
       : yetanotherthing > 2 ? 2
       :                       3; # default
于 2009-06-08T17:15:12.707 回答
0

我这样做的方式除了我之外没有人喜欢。我不在乎代码是否“看起来不错”。如果我在那个条件下得到了 >1 的测试,那意味着我可能想要更多,或者我可能想要评论其中的一些,我想简化这些更改的制作。

所以我这样编码:

if (true
  && test_1
  && test_2
  ...
  )
{
  ...
}

这使得注释掉测试或添加新测试变得容易,作为 1 行编辑。

但我会第一个承认,它并不声称漂亮。

于 2009-06-08T18:13:04.210 回答
0

像这样的东西

我会进一步解释一点。(并修复愚蠢的错误:S)

//Interface to be able to which classes are able to give a boolean result used in the if stuff
public interface IResultable
{
    bool Result();
}

//A list that is IResultable itself it gathers the results of the IResultables inside.
public class ComparatorList<T> : List<T>, IResultable where T : IResultable
{
    public bool Result()
    {
        bool retval = true;
        foreach (T t in this)
        {
            if (!t.Result())
            {
                retval = false;
            }
        }
        return retval;
    }
}

//Or two bools
public class OrComparator : IResultable
{
    bool left;
    bool right;

    public OrComparator(bool left, bool right)
    {
        this.left = left;
        this.right = right;
    }
    #region IResultable Members

    public bool Result()
    {
        return (left || right);
    }

    #endregion
}

// And two bools
public class AndComparator : IResultable
{
    bool left;
    bool right;

    public AndComparator(bool left, bool right)
    {
        this.left = left;
        this.right = right;
    }
    #region IResultable Members

    public bool Result()
    {
        return (left && right);
    }

    #endregion
}

// compare two ints
public class IntIsComparator : IResultable
{
    int left;
    int right;

    public IntIsComparator(int left, int right)
    {
        this.left = left;
        this.right = right;
    }
    #region IResultable Members

    public bool Result()
    {
        return (left == right);
    }

    #endregion
}

你有很多如果陈述这可能很酷:)

所以我们有这个结构来处理很多比较。我将给出一个小示例实现。

//list of ands
ComparatorList<AndComparator> ands = new ComparatorList<AndComparator>();    
ands.Add(new AndComparator(true,true));

//list of ors
ComparatorList<OrComparator> ors = new ComparatorList<OrComparator>();
ors.Add(new OrComparator(false, true));

//list of intiss
ComparatorList<IntIsComparator> ints = new ComparatorList<IntIsComparator>();
ints.Add(new IntIsComparator(1, 1));

//list of all things :)
ComparatorList<IResultable> collected = new ComparatorList<IResultable>();
collected.Add(ands);
collected.Add(ors);
collected.Add(ints);

// if all things are as they must be :)
if (collected.Result())
{
    //Evertything is as it schould be :)
}
于 2009-06-08T18:16:01.867 回答
0

在一个“if”中看到这么多子句并不常见。当您需要执行某些行而不管某些条件的真实性时,您通常会发现您需要嵌套“ifs”以获得所需的逻辑。我不是说如果你不需要嵌套它们,如果它们都需要同时进行测试。只有当有一些共同的功能。另一个考虑是设置一个布尔变量,其中包含一些这些条件的结果,这可能会更容易理解。如果你的变量是一个数组或集合,你能遍历它们吗?您是否针对-1测试它们?

于 2009-06-08T17:10:10.823 回答
0

您说您正在查看字符串,那么有人已经评论过这样的事情怎么样。

        var items = new List<string>();

        items.Add("string1");
        items.Add("string2");

        if (items.Contains("string2"))
        {
            // do something
        }

您甚至可以从某种配置文件中获取值来填充列表。

于 2009-06-09T08:27:46.270 回答
0

If you really have to check against all these conditions, you can't get rid of them. You can only refactor it and make it more readable, but the logic stays the same.

I'm quite surprised that no one has mentioned anything about redesigning your code. Do you really really need 20 differents states? In my experience, a lot of states are often depending on other states and are therefore often logically redundant to check against.

Refactoring your code might help you get a better understanding of it and how the states are coupled to each other. I'd start here first if I were you :)

于 2009-06-08T18:44:30.333 回答
0

虽然我喜欢达里奥的解决方案(正如我评论的那样,我将它放在一个布尔函数中,所以我不必在 if 的条件部分有一个新的......)我不确定有什么问题:

if((something == -1) &&
   (somethingElse == -1) &&
   (elseElse == -1) &&
   ...
  )

我认为这可能比我必须处理的许多 ((A && B) || (C &&(D||E))) 更容易阅读......

于 2009-06-08T17:59:45.933 回答