公共/私有方法的函数参数验证最佳实践

Question

经过一些研究，似乎人们普遍认为公共方法的参数应该被验证，而私有函数通常不需要。这让我有一些问题，但迄今为止我还没有找到一个令人满意的答案。

例子：

public void DoSomething(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double d = DoWork(i);
}

private double DoWork(int i)
{
    double ret = ...; // some calculation
    return ret;
}

想法:

1. 如果在DoWork()内部i的非负要求发生变化怎么办？设计风险会留下过时的验证检查。我知道程序员有责任调整已经改变的函数的使用方式，但我不禁想知道是否有更好的方法来最小化错误的风险。

2. 那么不是从DoSomething()调用DoWork()的不同调用呢？我们必须冗余地验证参数吗？

public void DoSomething(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double d = DoWork(i);
}

public void DoSomethingElse()
{
    int i = 5;
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double d = DoWork(i);
}

private double DoWork(int i)
{
    double ret = ...; // some calculation
    return ret;
}

通过将检查放入自己的函数中，可以稍微简化这个过程。然后存在一个风险，即调用DoWork(int i)的新函数会忘记验证i。

public void DoSomething(int i)
{
    ThrowIfIntegerIsNegative(i);

    double d = DoWork(i);
}

public void DoSomethingElse()
{
    int i = 5;
    ThrowIfIntegerIsNegative(i);

    double d = DoWork(i);
}

static void ThrowIfIntegerIsNegative(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");
}

private double DoWork(int i)
{
    double ret = ...; // some calculation
    return ret;
}

这个比那个更好吗？

public void DoSomething(int i)
{
    double d = DoWork(i);
}

public void DoSomethingElse()
{
    double d = DoWork(5);
}

private double DoWork(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double ret = ...; // some calculation
    return ret;
}

根据情况，这是我同时努力实现的一些目标：

1. 在单个位置（可能在使用参数的函数内部）进行参数验证
2. 尽早报告错误，而不是等到最后才因为错误的用户输入而导致大量代码执行失败
3. 避免多次验证参数
4. 避免对发布代码造成性能影响

如何取得平衡？哪种方法对您最有效？我将非常感激任何见解。

- Daniel McClelland

---

CodeContracts会有所帮助。 - Bob Vale

@BobVale 代码必须通过Mono进行移植，因此我不确定Code Contracts是否得到了完全支持（http://www.mono-project.com/Compatibility）。我记得在某个地方读到过使用Spec#会将代码与Visual Studio耦合在一起，这对现在来说可能不是问题，但未来很可能会成为问题。 - Daniel McClelland

Code Contract的Contract.Requires()前置条件在Mono 2.8中得到支持，同时还有一个代码重写器来正确处理它们 - 这本身就非常有用。MS版本有一个设置，控制是否对公共+私有方法进行仪器化，或者仅对公共方法进行仪器化，并且您可以在调试和发布版本之间不同地选择。我希望Mono版本也支持这一点。 - Matthew Watson

Answer 1

在公共方法中验证参数的逻辑是这样的，在私有方法中不验证参数：

• 当使用无效参数调用公共方法时，这是一个你无法控制的编程错误。
• 当使用无效参数调用非公共方法时，则是你可以控制的逻辑错误。

这种逻辑是合理的：没有必要在验证模块内部生成的方法的参数上浪费周期。另一方面，私有方法始终可以假定它们的参数是有效的，因为你可以控制所有对私有方法的调用。

然而，捕获这些假设被违反的情况非常有益。为此，在私有方法内部使用运行时断言代替参数验证是一个非常好的想法。这将通过异常捕获来捕获外部调用者的无效调用，并使用asserts警告你自己的方法的无效调用。

- Sergey Kalinichenko

---

我对你最后一段有些困惑。在我看来，参数验证和运行时断言可能是相同的。这些断言可以采用if语句的形式，或者使用Debug.Assert()。你想表达什么意思？ - Daniel McClelland

@DanielMcClelland 断言（例如 Debug.Assert）和运行时参数检查的关键区别在于它们向代码读者传达了不同的信息：检查参数并抛出异常表示调用者可能会传递错误的参数；而断言则告诉读者您已确保参数是有效的。异常是为您的调用者准备的，而断言是为您自己准备的。从某种意义上说，断言几乎像是“私有异常”：它们的目的是告诉您，在某些情况下，您对代码的假设是错误的。 - Sergey Kalinichenko

@DanielMcClelland Jon Skeet写了一篇精彩的答案，详细介绍了断言与异常之间的区别。他还提到，应该使用断言来检查私有方法的参数。 - Sergey Kalinichenko

Answer 2

我喜欢在公共方法中使用异常进行验证：

public void Foo(int i)
{
    if (i < 0)
      throw new ArgumentOutOfRangeException("i");

    double d = DoWork(i);
}

我喜欢在私有方法中使用断言进行验证：

private double DoWork(int i)
{
    System.Diagnostics.Debug.Assert(i >= 0);
    // ....
}

1. 这意味着在多个公共方法中进行重复验证。
2. 尽早捕获无效参数。
3. 由于参数名称等方面的差异，因为私有方法中的无效参数而引发ArgumentOutOfRangeException异常可能会令调用者感到困惑。
4. 通常，您的公共方法在调用私有方法之前会执行额外的工作，因此可以节省该项工作。
5. 使用单元测试确保新方法不会错过任何验证检查。

另一个需要做的是将任何受保护的方法视为公共方法。

编辑：

这是违反上述第3点的错误验证示例：

public void Foo(int distanceInMetres)
{
    double d = DoWork(distanceInMetres * 1000);
}

private double DoWork(int distanceInMillimetres)
{
    if (distanceInMillimetres < 0)
      throw new ArgumentOutOfRangeException("distanceInMillimetres");
    // ....
}

如果调用者在参数“distanceInMillimetres”上看到了异常，他会感到困惑，因为他调用了一个名为“distanceInMetres”的参数的“Foo”函数。

- Matt Houser

Answer 3

赞同dasblinkenlight关于公共方法和非公共方法的说法。

如果您练习TDD，那么您的私有方法只是重构代码并将一些公共方法的片段提取到私有方法中的结果。因此，如果您用测试覆盖了公共方法，您的私有方法将自动被测试。

如果您不使用TDD但想验证逻辑是否被违反，您可以使用不同的技术，例如-断言(http://msdn.microsoft.com/en-us/library/ttcc4x86.aspx)或代码契约(http://msdn.microsoft.com/en-us/library/dd264808.aspx)。

- rpeshkov