我该如何在C#中重写一个非常大的复合if语句？

Question

我该如何在C#中重写一个非常大的复合if语句？

c#refactoringstructureif-statementguard-clause

26

在我的C#代码中，我有一个if语句，最开始看起来很无害：

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

它在不断增长。我认为现在里面肯定有20个条款。

我应该如何处理这个问题？

- Michael Broschat

我应该提供更多的信息，我明白了。我对回复的数量感到非常惊讶，需要一些时间来仔细阅读它们。我正在检查是否可以运行程序的主要代码。除非我正在查看的字符串中包含某些内容，否则我会运行它。我通过经验发现这些内容。每次发现一个，我就添加一个子句。再次感谢所有的帮助... - Michael Broschat

在这种情况下，为什么不通过一些方式（如配置文件）创建一个List<String> 并在foreach中进行比较呢？你可以将其作为守卫条款，一旦找到不喜欢的内容，就立即返回。 - aehiilrs

18个回答

18

将其分解为一个函数，并将每个条件作为守卫子句：

int maybe_do_something(...) {
    if(something != -1)
        return 0;
    if(somethingelse != -1)
        return 0;
    if(etc != -1)
        return 0;
    do_something();
    return 1;
}

- chaos

1

从代码角度来看，这与嵌套的if语句方法没有区别。相反，我会尝试将标准分离成更易管理的方法，每个方法只检查一些标准，然后“最内层”的方法将执行实际代码。这将使代码更具可重用性和易于维护。 - Lasse V. Karlsen

5

是的，这样做可能有些冗余。但是编程不仅是为了让计算机理解，还要考虑其他人阅读和维护你所写的代码。所以为了可读性而言，上述改动是很有必要的。 - J. Polfer

我不这么认为。如果将原始的if语句分成多行，它看起来会更好。 - Dario

请参阅我对Brian Postow答案的评论。 - chaos

1

多余的？因为它有不止一个“return”吗？我认为你不知道这个术语的含义。 - user1228

17

假设所有这些条件都是必需的，您可以将这些条件合并为一个或多个超级布尔值或函数调用，以提升可读性。

例如，

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

if (TeamAIsGoForLaunch && TeamBIsGoForLaunch && TeamC.isGoForLaunch())

- steamer25

这个if语句中应该有语义选项组。请尝试重写代码以在if语句周围实现，并在可能/必要的情况下将它们制作为子函数（例如，如果它们将被重复使用）。 - Alex Feinman

1

我通常采用这种方法。这比其他任何解决方案更易读，因为您可以命名条件，并且它提供了更轻松的调试，因为您可以在到达if语句之前查看每个条件的值。 - Chap

@aehiilrs：等一下..这不是吗？哦...我的词汇障碍！B-) - Brian Postow

9

考虑一下为什么你有这么多子句。就像SWITCH语句经常表明您应该将选择移动到子类中一样，大量复杂的IF语句链可能表明您正在将太多概念（因此决策）合并到一个地方。

以计算佣金为例。在我建立的一个系统中，佣金率取决于给予批发商的佣金金额，批发商再将其传递给零售商。从批发商到零售商的给予金额取决于零售商和批发商之间的具体协议（基于合同）。批发商获得的金额也取决于产品线、具体产品和销售数量。

除此之外，还有基于客户所在州、特定产品定制等的额外“异常”规则。这些可以通过复杂的IF语句链来解决，但我们选择了使应用程序数据驱动。

通过将所有信息放入表格中，我们随后能够进行数据规则通行证。首先，批发商的通用规则将触发，然后任何覆盖规则都将触发。然后，在手头拥有基础批发商佣金后，我们将运行批发商到零售商的通用规则，然后是这些规则的异常情况。

这将一个巨大的逻辑混乱转化为一个简单的四个步骤过程，每个步骤只需进行数据库查询以找到正确的规则应用。

当然，这可能不适用于您的情况，但通常这样的大型复杂性意味着要么没有足够的类来分配责任（我们问题的另一个解决方案可以是包含特定规则和覆盖的工厂类），要么功能应该是数据驱动的。

- Godeke

1

编辑：如果您不想使用完整的规则系统或类集，Steamer的建议是一个好主意：将各种小条件组合成“超级条件”，可以真正清理代码并使后面的if语句更易于阅读。 - Godeke

这个简单的四步骤过程将原本复杂混乱的逻辑变成了一个清晰明了的解决方案。我更愿意说，它将原本存在于代码中的庞大逻辑问题转移到了规则表中。 - Anton Tykhyy

@AntonTykhyy 你为什么更愿意这样说呢？通过将逻辑分成四个阶段（每个阶段分为默认规则和潜在的覆盖规则），对于维护佣金系统的人来说，每个阶段都变得更加简单易懂，因为它现在反映了用户使用系统的心理模型。现在用户可以查询场景的规则系统，查看触发规则并知道为什么它们被触发。在基于代码块的逻辑中，没有任何可用的东西。显然，一个构造不良的规则系统不会有所帮助，反而可能会使情况变得更糟。不要那样做。 - Godeke

我明白了。我还没有考虑触发器的可见性角度。谢谢！ - Anton Tykhyy

8

将其重构为一个函数。

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

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

- J.W.

那么，“我应该如何处理我的大型if语句？”的答案就是将其移到一边吗？这几乎听起来不像是一个解决方案，不是吗？正确处理类似于此的大型条件块的方法是将它们拆分成可管理的块。 - Lasse V. Karlsen

1

@Lasse- 这取决于情况。有时问题是这个布尔逻辑做什么？使用提取方法可以使布尔逻辑的功能自我描述-http://www.refactoring.com/catalog/extractMethod.html。 - RichardOD

6

有很多处理这种情况的方法，但我挑选了一些。

首先，如果所有条件（if语句中的所有AND条件）+要执行的代码都是一次性的情况，则使用您已经拥有的代码。您可能想要做一些其他人已经建议的事情，重写以使用Guard-clause类型的代码。

换句话说，不要像这样：

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

我认为第二种方法比第一种更易读，更易重用。

- Lasse V. Karlsen

4

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

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

*编辑：给数据添加一个额外的行：

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

- Dario

我可能会将这个放在一个函数或宏中，以避免在if条件语句内调用new... - Brian Postow

3

您也可以将状态存储在一个类中。

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

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

- Brian

3

看起来你有三个信息，这些信息共同代表了应用程序中的一个特定状态。与其在这三个状态上进行切换，为什么不创建一个封装它们的值呢？然后你可以在创建时使用对象层次结构或委托来绑定你想要运行的操作。

- JaredPar

3

你可以将它重构为一个函数，并返回代表正确情况的枚举值：

if(something != -1)
    return MyEnum.Something;
if(somethingelse != -1)
    return MyEnum.SomethingElse;
if(etc != -1)
    return MyEnum.SomethingElseEntirely;
return MyEnum.None;

- Mark Carpenter

网页内容由stack overflow 提供, 点击上面的

可以查看英文原文，
原文链接

- user1228 · Accepted Answer

尽可能使用门。

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