是否将前提条件分组为可以保持DRY的方法？

Question

在为具有相似参数的不同函数编写前置条件时,我想将断言或异常分组到静态方法中,而不是明确地将它们写出来.例如,而不是

GetFooForUser(User user) 
{
  assert(null != user);
  assert(user.ID > 0); // db ids always start at 1
  assert(something else that defines a valid user);
  ...

  // do some foo work
}

GetBarForUser(User user) 
{
  assert(null != user);
  assert(user.ID > 0); // db ids always start at 1
  assert(something else that defines a valid user);
  ...

  // do some bar work
}

我更愿意写

GetFooForUser(User user) 
{
  CheckUserIsValid(user);

  // do some foo work
}

GetBarForUser(User user) 
{
  CheckUserIsValid(user);

  // do some bar work
}

static CheckUserIsValid(User user)
{
  assert(null != user);
  assert(user.ID > 0); // db ids always start at 1
  assert(something else that defines a valid user);
  ...
}

这感觉更自然,有助于减少我需要编写的代码(甚至可能减少我的断言中的错误数量!)但似乎违背了前提条件应该有助于记录方法的确切意图的想法.

这是一种常见的反模式还是有任何重要原因不这样做？

另外,如果我使用了异常,答案会有所不同吗？

Answer 1

可靠的答案

这是绝对可以接受的：.NET 源代码包装了条件。

例如，StringBuilder源代码中有一个名为的方法VerifyClassInvariant()，它调用了 18 次。该方法仅检查正确性。

private void VerifyClassInvariant() 
{    
    BCLDebug.Correctness((uint)(m_ChunkOffset + m_ChunkChars.Length) >= m_ChunkOffset, "Integer Overflow");
    StringBuilder currentBlock = this;
    int maxCapacity = this.m_MaxCapacity;
    for (; ; )
    {
        // All blocks have copy of the maxCapacity.
        Contract.Assert(currentBlock.m_MaxCapacity == maxCapacity, "Bad maxCapacity");
        Contract.Assert(currentBlock.m_ChunkChars != null, "Empty Buffer");

        Contract.Assert(currentBlock.m_ChunkLength <= currentBlock.m_ChunkChars.Length, "Out of range length");
        Contract.Assert(currentBlock.m_ChunkLength >= 0, "Negative length");
        Contract.Assert(currentBlock.m_ChunkOffset >= 0, "Negative offset");

        StringBuilder prevBlock = currentBlock.m_ChunkPrevious;
        if (prevBlock == null)
        {
             Contract.Assert(currentBlock.m_ChunkOffset == 0, "First chunk's offset is not 0");
             break;
        }
        // There are no gaps in the blocks. 
        Contract.Assert(currentBlock.m_ChunkOffset == prevBlock.m_ChunkOffset + prevBlock.m_ChunkLength, "There is a gap between chunks!");
        currentBlock = prevBlock;
    }
}

对话

是否可以将断言或异常分组到静态方法中，而不是显式地将它们写出来？

是的。没关系。

...这似乎违背了先决条件应该帮助记录方法的确切意图的想法。

assert包装or语句的方法名称Exception应该传达包装器的意图。另外，如果读者想了解细节，那么她可以查看该方法的实现（除非它是闭源的。）

这被认为是好的做法还是坏的做法，为什么？

将一组相关的或经常重用的方法包装在另一个方法中是一个很好的做法asserts，因为它既可以提高可读性，又可以方便维护。

这是常见的反模式吗？

事实上恰恰相反。您可能会看到类似的内容，它实际上很有帮助并值得推荐，因为它沟通良好。

private void IfNotValidInputForPaymentFormThenThrow(string input)
{
    if(input.Length < 10 || input.Length)
    {
        throw new ArgumentException("Wrong length");
    }

    // other conditions omitted
}

最好添加ThenThrow到方法的末尾，以便调用者知道您正在抛出。否则，您可能希望返回 abool并让调用者决定如果条件失败该怎么办。

private bool IsValidInputForPaymentForm(string input)
{
    if(input.Length < 10 || input.Length)
    {
        return true;
    }
    else
    {
        return false;
    }

    // other conditions omitted
}

然后调用代码可以抛出：

if(!IsValidInputForPaymentForm(someStringInput)
{
    throw new ArgumentException();
}

或者，这是来自 .NET 源代码的另一个示例，如果某些条件失败，该示例将引发异常（ThrowHelper仅引发异常。）

// Allow nulls for reference types and Nullable<U>, 
// but not for value types.
internal static void IfNullAndNullsAreIllegalThenThrow<T>(
    object value, ExceptionArgument argName) 
{
    // Note that default(T) is not equal to null 
    // for value types except when T is Nullable<U>. 
    if (value == null && !(default(T) == null))
        ThrowHelper.ThrowArgumentNullException(argName);
}

有什么重要的理由不这样做吗？

我能想到的是，如果方法名称没有解释您要检查的内容，并且没有一种简单的方法来查看源代码，那么您可能应该避免包装条件。