【问题标题】:Is using "out" bad practice正在使用“out”的糟糕的练习
【发布时间】:2011-05-14 09:37:47
【问题描述】:

我刚刚在我编写的方法中添加了一个 out bool 参数,以便在我的 UI 中获得警告。我使用了 out 而不是让方法本身返回 false/true,因为这意味着 DoSomething 失败/成功。我的想法是 warnUser 将指示警告实际上是什么,而无需查看方法的实现。

原始代码

public void DoSomething(int id, string input);

新代码

public void DoSomething(int id, string input, out bool warnUser);

我正在使用 Moq 来测试这段代码,但它不支持 out/ref 参数,因为 Lambda 表达式不支持它们

测试代码

mockService.Verify(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>());

那么,使用 out 参数是不好的做法,如果是,我该怎么做?

【问题讨论】:

  • bool“如何指示警告的实际内容?”
  • @Cody - 因为它在我的“真实”代码中是一个有意义的名称 :)

标签: c# lambda moq anti-patterns


【解决方案1】:

在 void 方法中使用 out 参数通常是个坏主意。您说您已经使用它“而不是让方法本身返回 false/true,因为这意味着 DoSomething 失败/成功” - 我不相信存在这种含义。通常在 .NET 中,失败是通过异常而不是真/假来指示的。

out 参数通常比返回值更难用——特别是,你必须有一个正确类型的变量来处理,所以你不能只写:

if (DoSomething(...))
{
   // Warn user here
}

您可能要考虑的一种替代方法是使用枚举来指示所需的警告级别。例如:

public enum WarningLevel
{
    NoWarningRequired,
    WarnUser
}

然后该方法可以返回WarningLevel 而不是bool。这会让你的意思更清楚——尽管你可能想稍微重命名一些东西。 (虽然我完全理解你为什么在这里使用元句法名称,但很难给出建议。)

当然,另一种选择是您可能希望显示更多信息 - 例如警告的原因。这可以通过枚举来完成,或者您可能希望完全给出更丰富的结果。

【讨论】:

  • 我不同意失败的含义。如果一个方法有一个布尔返回,对我来说这意味着返回 false 意味着它失败了。但我同意失败通常由抛出异常(最好是强类型异常)表示。虽然它确实喜欢枚举的想法,但我应该认识到我自己过去曾使用过这种方法。谢谢。
  • +1 表示WarningLevel 建议。这将消除询问者所担心的关于返回值被解释为函数成功或失败的混淆。
  • 我已经接受了这个答案,因为我的调用代码将更具可读性。 "if (!DoSomething(...)" vs "if (DoSomething(...) == WarningLevel.WarnUser)"。显然我会有比 "WarnUser" 更有意义的东西
【解决方案2】:

out非常一个有用的构造,尤其是在像bool TryGetFoo(..., out value) 这样的模式中,你想知道“如果”和“什么”分开(并且可以为空不一定是一种选择)。

但是 - 在这种情况下,为什么不直接做到:

public bool DoSomething(int id, string input);

并使用返回值来表示这一点?

【讨论】:

  • 正如我所说,我不想暗示“某事”失败/成功
  • @Marc Gravell 我认为在 .NET 4 中,使用 Tuple 可能会更清晰,因为输入(方法参数)与输出(返回值)分开并且仍然提供“如果”和“什么”。当然,元组在 lambda 中也能正常工作。
  • @Antony - 我不确定我是否看到了该评论的重要性;但是,您确实似乎只返回了一个值 - 当您可以使用 return 时,为什么还要使用 out
  • @Antony - 然后考虑重命名该方法,使其确实有意义;或者,坦率地说,克服它;p 返回值并不意味着“失败”(这不是 C/C++)——它意味着“这里是返回值”。它是bool 的事实无关紧要。
  • @Antony(Exception 抛出意味着失败)
【解决方案3】:

一些额外的想法:

  • FxCop 所说:CA1021: Avoid out parameters

  • 对于私有/内部方法(实现细节)out/ref 参数不是问题

  • C# 7 Tuples 通常是out 参数的更好替代方案

  • 此外,C# 7 通过引入“输出变量”和允许“丢弃”out 参数改进了调用站点上对 out 参数的处理

【讨论】:

    【解决方案4】:

    给定一个大的遗留代码方法(比如 15 行以上,200 多行的遗留代码方法很常见),人们可能希望使用“提取方法”重构来清理代码并使代码不那么脆弱和更易于维护。

    这样的重构很可能会产生一个或多个没有参数的新方法,用于将变量值设置为先前方法的局部变量。

    就我个人而言,我使用这种情况来强烈指示在先前的方法中隐藏了一个或多个离散类,以便我可以使用“提取类”,在新类中,这些输出参数获得可见的属性调用(之前的)方法。

    我认为在前一个方法中保留没有参数的提取方法是不好的做法,跳过“提取类”的连贯后续步骤,因为如果你跳过它,那些局部变量会保留在你之前的函数中,但在语义上不再属于它。

    因此,在这种情况下,我认为 out 参数是破坏 SRP 的代码气味。

    【讨论】:

      【解决方案5】:

      如果不了解更多上下文,这是一个很难回答的问题。

      如果DoSomething 是 UI 层中的一个方法,它是做一些与 UI 相关的事情,那么也许没问题。如果DoSomething 是业务层方法,那么这可能不是一个好方法,因为这意味着业务层需要了解可能的适当响应,甚至可能必须注意本地化问题。

      就纯粹主观而言,我倾向于远离参数。我认为他们扰乱了代码的流程,让代码变得不那么清晰。

      【讨论】:

        【解决方案6】:

        我认为最好的方法是使用预期的异常。您可以创建一组自定义异常来定义您的警告,并在调用方法中捕获它们。

        public class MyWarningException : Exception() {}
        
        ...
        
        try
        { 
             obj.DoSomething(id,input);
        }
        catch (MyWarningException ex)
        {
             // do stuff
        }
        

        这个想法是在 DoSomething 实现结束时引发异常,以便流程不会在中间停止

        【讨论】:

        • 只有当方法不能完成它的意图时才应该抛出异常,IMO。如果它成功了,但由于某种原因应该给用户一个警告,这对我来说听起来不是一个例外情况。
        • @ykatchou 实际上并没有那么慢;但对于一般逻辑流程来说,这不是一个好习惯。
        • 我不能使用异常,因为它会对调用 DoSomething 方法的 UI 代码产生不必要的副作用。
        • 乔恩,异常表示一种行为。如果您期待这种行为,那么这并不意味着该方法无法完成它的工作??
        • 有什么副作用?如果您正在处理特定的异常,则没有副作用。如果异常是其他类型(无论是什么),那么它将像往常一样在调用堆栈中冒泡
        【解决方案7】:

        我认为最适合您的情况的最佳解决方案是使用委托而不是布尔。使用这样的东西:

        public void DoSomething(int id, string input, Action warnUser);
        

        您可以在不想向用户显示警告的地方传递一个空的 lamda。

        【讨论】:

          猜你喜欢
          • 1970-01-01
          • 1970-01-01
          • 2014-09-07
          • 2010-11-15
          • 1970-01-01
          • 2022-07-13
          • 2020-03-31
          • 2012-12-19
          • 1970-01-01
          相关资源
          最近更新 更多