6

同様のパラメーターを持つさまざまな関数の前提条件を記述する場合、アサーションまたは例外を明示的に記述するのではなく、静的メソッドにグループ化したいと考えています。たとえば、代わりに

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

これはより自然に感じられ、記述する必要のあるコードを削減するのに役立ちます (そして、アサーションのエラーの数を減らすことさえあります!) が、前提条件がメソッドの正確な意図を文書化するのに役立つはずであるという考えに反するようです.

これは一般的なアンチパターンですか、それともこれを行わない重大な理由はありますか?

また、例外を使用した場合、答えは異なりますか?

4

1 に答える 1

2

確かな答え

.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;
    }
}

ダイアログ

アサーションまたは例外を明示的に記述するのではなく、静的メソッドにグループ化しても問題ありませんか?

はい。それは大丈夫。

...前提条件がメソッドの正確な意図を文書化するのに役立つはずであるという考えに反しているようです。

assertorステートメントをラップするメソッドの名前はException、ラッパーの意図を伝える必要があります。また、読者が詳細を知りたい場合は、メソッドの実装を表示できます (クローズド ソースでない限り)。

これは良い習慣か悪い習慣か、またその理由は?

関連する、または一般的に再利用される一連のメソッドを別のメソッドでラップすることをおasserts勧めします。これにより、読みやすさが向上し、メンテナンスが容易になります。

これは一般的なアンチパターンですか？

実際には反対です。あなたはこのようなものを見るかもしれません.

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

    // other conditions omitted
}

ThenThrow呼び出し元がスローしていることを認識できるように、メソッドの最後に追加することをお勧めします。それ以外の場合は、 a を返してbool、条件が失敗した場合の処理​​を呼び出し元に決定させることができます。

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

これを行わない重大な理由はありますか？

私が考えることができるのは、メソッド名がチェックしているものを説明しておらず、ソースを表示する簡単な方法がない場合は、おそらくラッピング条件を避けるべきだということです。

于 2015-04-30T23:00:31.137 に答える