オブジェクト指向スタイルのパラメーターと型パラメーター

StackOverflow https://stackoverflow.com/questions/403058

  •  03-07-2019
  •  | 
  •  

質問

次の2つの方法があるとします：

番号1：

void AddPerson(Person person)
{
  // Validate person
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

番号2：

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  DB.AddPersonToDatabase(person);
}

2つの方法のうち、どれが最適ですか？私は最初のものがオブジェクト指向の方が正しいことを知っていますが、2番目のものはより読みやすいと感じています。パラメータがこれを確認するので、オブジェクトが有効であることを確認する必要はありません。オブジェクトをパラメーターとして渡す場所を検証する必要はありません。他のアプローチはありますか？

編集： すべての答えのためのThx。明確にするために、コンストラクターとIsValidメソッドで検証することはもちろん良いアプローチですが、私のコードでは、人の有効な状態はコンテキストに依存することが多く、メソッドによって異なる場合があります。もちろん、これはデザインが悪い兆候かもしれません。

コードは、問題を説明するための単なる例です。

役に立ちましたか？

解決

最初の人はperson.Nameとperson.BirthDateを検証する必要はありません-それらは Person コンストラクターによって自動的に検証されるべきです。つまり、Personが渡された場合、それが有効であることを知っておく必要があります。

一方、 person がnull参照ではないことを確認する必要があります。

コンストラクターを頻繁に明示的に呼び出さなくても済むように、2番目のバージョンのような便利なメソッドを用意する価値がある場合があります。通常は、 Person コンストラクターを呼び出してから、作業を最初のフォームに委任するだけです。

他のヒント

最初の方法には、既存のコードを壊すことなくPerson定義を変更できるという利点があり、再コンパイルのみが必要です。 2番目の方が読みやすいと思うかもしれませんが、1番目の方がより保守しやすいと思います。

別のオプションは次のとおりです。

void AddPerson(Person person)
{  // Validate person  
   if(person.IsValid)
   {
     DB.AddPersonToDatabase(person);
   }
}

Personは、構築時に自身を検証しないと仮定します。オブジェクトが一時的でありながら無効な状態になる可能性がある場合、場合によっては実行可能です。

前者（オブジェクトを渡す）の方が好みです。APIのオブジェクトへの結合を減らすからです。 Person オブジェクトを変更した場合、たとえば以前は必要なかった Nickname などの新しいプロパティを追加し、最初の場合はパブリックAPIを変更する必要はありませんが、2番目の場合はメソッドを変更するか、新しいオーバーロードを追加します。

完全にコンテキストに依存することに同意します。これには絶対的なルールはありません。私の意見では、次のようなメソッドを持つのはナンセンスです：

person.SetBirthDate(Person person)
person.ResetPassword(Person person)

しかし、この場合、前者の方が好きです。GregBeechが言ったように、このメソッドはドメインオブジェクトについて何も知らない（必要がある）からです。

ところで、オーバーロード（DRY-繰り返さないでください）：

void AddPerson(Person person)
{
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  this.AddPerson(p);
}

多数のプリミティブ型をパラメーターとして渡すよりも、Personオブジェクトを渡す方が間違いなく優れています。次の2つの方法を比較します


public static void Withdrawal(Account account, decimal amount)
{
    DB.UpdateBalance(account.AccountNumber, amount);
}

public static void Withdraw(int accountNumber, decimal amount)
{
    DB.UpdateBalance(accountNumber, amount);
}

2つの方法はほとんど同じように見えますが、2番目の方法は安全ではありません。 intはどこからでも来ることができるので、これを書くとめちゃくちゃになります：

private void CloseTransaction(Transaction tran)
{
    BankAccounts.Withdrawal(tran.Account.RoutingNumber, tran.Amount);
        // logic error: meant to pass Account.AccountNumber instead of Account.RoutingNumber
}

これは、コンパイルエラーまたはランタイム例外をスローしないため、最悪の種類のエラーです。自動テストで十分に記述すれば、このエラーをキャッチできますが、このバグは簡単に見逃され、発見されずに数か月間隠れる可能性があります。

私は銀行のソフトウェアを書いている会社で働いていましたが、実際にこの種のバグに出くわしました。特定の種類のボールト転送中にのみ発生し、月末にプロセスを実行するたびに、銀行の1つが数百ドルずつGL残高に気づいたときにのみ発見されました。銀行は従業員の盗難を数か月間疑っていましたが、誰かが問題をソフトウェアのバグにたどったのは、慎重なコードレビューを通してのみでした。

ほとんどの場合、最初のものに行きます。

2番目のものは、Personのすべての変更に対して署名を破ります

コンテキストに依存します。

すべての呼び出しメソッドがPersonオブジェクトを処理する場合、最初は正しい解決策です。

ただし、呼び出しメソッドの一部が名前と誕生日を処理し、一部がPersonオブジェクトを処理する場合、2番目の例は正しい解決策です。

両方のオーバーロードを作成します。特に、自動的に作成できることを考慮してください。

メソッドは Person タイプの一部ではないと想定しています。それを念頭に置いて、私は両方とも別のタイプ（ Person ）imoに関する知識が少なすぎると感じています。最初のバージョンでは、有効な人物インスタンスの作成を検証する必要はありません。これが呼び出し側の責任である場合、すべての呼び出し側はこれを行わなければなりません。それはもろい。

2番目のバージョンは、このタイプのインスタンスを作成するという事実により、別のタイプに強い依存関係があります。

私は間違いなく最初のバージョンを好むでしょうが、コードのその部分から検証部分を移動します。

ジョンはそれをほとんど打ち付けたと思います。 Person は、コンストラクターで有効なPersonが作成されるようにする責任があります。

Person オブジェクト（AddPersonメソッドまたはその呼び出し元）を作成するかどうかを決定する責任について、読み取り

http://en.wikipedia.org/wiki/GRASP_%28Object_Oriented_Design ％29＃Creator

OOPにおける責任の問題についてです。あなたの特定のケースでは、AddPersonがDBインターフェースへの呼び出しをラップしている場合、それについてはよくわかりません。それは、そのPersonオブジェクトがそのコンテキストの外部で使用されるものに依存します。データベースに追加するデータを含めることのみを目的とする場合は、AddPersonメソッドで作成することをお勧めします。これは、クラスのユーザーをPersonクラスについて知る必要がなくなるためです。 。

オブジェクトを使用する場合のカプセル化の改善。それが彼らの目的です。プリミティブを使いすぎると人々はトラブルに巻き込まれると思います。

無効な人物を作成することはできません。コンストラクターは、有効なパラメーターを確認し、IllegalArgumentExceptionをスローするか、無効な場合はアサートに失敗する必要があります。これが、契約によるプログラミングのすべてです。

ライセンス： CC-BY-SA と 帰属
所属していません StackOverflow
scroll top