3

このクラスがあると想像してください

class Ai1ec_Less_Parser_Controller {
    /**
     * @var Ai1ec_Read_Variables_Startegy
     */
    private $read_variable_strategy;
    /**
     * @var Ai1ec_Save_Variables_Strategy
     */
    private $write_variable_strategy;
    /**
     * @var Ai1ec_Less_Variables_Collection
     */
    private $less_variables_collection;
    /**
     * @var Ai1ec_Less_Parser
     */
    private $ai1ec_less_parser;
    /**
     * We set the private variables in the constructor. I feel that there are too many parameters. 
     * Should i use setter instead and throw an exception if something is not set?
     * 
     * @param Ai1ec_Read_Variables_Startegy $read_variable_strategy
     * @param Ai1ec_Save_Variables_Strategy $write_variable_strategy
     * @param Ai1ec_Less_Variables_Collection $less_variables_collection
     * @param Ai1ec_Less_Parser $ai1ec_less_parser
     */
    public function __construct( Ai1ec_Read_Variables_Startegy $read_variable_strategy,
                                 Ai1ec_Save_Variables_Strategy $write_variable_strategy,
                                 Ai1ec_Less_Variables_Collection $less_variables_collection,
                                 Ai1ec_Less_Parser $ai1ec_less_parser ) {

    }
}

これらの変数を設定する必要があるため、コンストラクターで設定します (ただし、パラメーターが多すぎるように見えます)。別のオプションは、セッターを使用してそれらを設定し、必要な変数の1つがこのように設定されていない場合、メソッドで例外をスローすることです

public function do_something_with_parser_and_read_strategy() {
  if( $this->are_paser_and_read_strategy_set === false ) {
     throw new Exception( "You must set them!" );
  }
}  
private function are_paser_and_read_strategy_set () {
  return isset( $this->read_variable_strategy ) && isset( $this->ai1ec_less_parser );
}

2 つの方法のどちらかが優れていると思いますか?その理由は?

4

6 に答える 6

2

あなたのクラスは不変ですか？もしそうなら、コンストラクターを介して 100% のメンバー人口を持つことが最善の方法であることが多いですが、5 つまたは 6 つを超えるパラメーターがあると見苦しくなり始める可能性があることに同意します。

クラスが変更可能な場合、必須パラメーターを持つコンストラクターを使用してもメリットはありません。アクセサー/ミューテーター メソッド (別名プロパティ) を介してメンバーを公開します。

ファクトリ パターン (@Ray によって提案されている) が役立ちますが、同様のさまざまなクラスがある場合に限ります。1 回限りの場合は、静的メソッドを使用してオブジェクトをインスタンス化できますが、「多くのパラメータ」の問題。

最後の代替手段は、フィールド (パラメーターごとに 1 つのフィールド) を持つオブジェクトを受け入れることですが、この手法は慎重に使用してください。一部の値がオプションである場合は、メソッドのオーバーロードを使用してください (残念ながら PHP はサポートしていません)。

私はあなたがしていることに固執し、問題が発生した場合にのみ他のものに変更します.

于 2012-07-17T00:12:35.207 に答える
1

クラスの命名 Controller は、どういうわけかMVC、または一般的には、シーケンスの処理を担当するメカニズムを反映しています。

いずれにせよ、データオブジェクトクラスは多くのフィールドを持つ傾向があります - それは彼らの責任です。他の多くのオブジェクトに依存する通常のオブジェクトは、ポイントを逃している可能性があります。

私が見るように、4 つのオブジェクトがあります: 読み取り、保存、解析、および何かへのコレクション インターフェイスの提供です。読み書き用に異なるインターフェイスを使用する必要があるのはなぜですか? これは一つにまとめられませんか？パーサーはそれ自体がライブラリであるため、それを組み合わせて使用​​する理由はどこにもないかもしれませんが、パーサー自体にリーダー/ライターを使用し、代わりにコレクションを提供することはできます。したがって、パーサーがリーダーの引数を取り、コレクション オブジェクトを返す可能性はありますか?

それは特定のケースについての詳細です。一般に、メソッドに多くの引数がある (または、異なるドメインの他のオブジェクトによってオブジェクト内の多くのフィールドを初期化する) ことは、ある種の設計上の欠陥を示しています。

トピックの種類は、コンストラクターの初期化に関するこの記事かもしれません-コンストラクター内の初期化を使用することをお勧めします。要点まで必ずフォローアップしてください。

コンストラクターで提供するコラボレーターが多数ある場合はどうなりますか? 大きなパラメータ リストと同様に、構築パラメータの大きなリストはCodeSmellです。

Rayが書いたように、セッターを使用して初期化する可能性があり、それに関する記事もあります。私の見解では、Martin Fowler はこれらの事例を非常にうまくまとめていると思います。

于 2012-07-18T09:00:41.170 に答える
1

「より良い」方法はありません。ただし、考慮すべき点がいくつかあります。

  • コンストラクターは継承されません
  • クラスがあまりにも多くのオブジェクトを必要とする場合、それはあまりにも多くの責任があります

これは、クラスが実装するインターフェイスの種類の選択に影響を与える可能性があります。

一般的な経験則は次のとおりです。

クラスが機能するためにパラメーターが必須である場合は、コンストラクターを介して注入する必要があります。

例外は、ファクトリを使用してインスタンスを初期化する場合です。ファクトリがさまざまなクラスからインスタンスを構築することは非常に一般的であり、それらのいくつかは同じインターフェイスを実装したり、同じ親クラスを拡張したりします。そうすれば、セッターを介して共有オブジェクトを注入する方が簡単です。

于 2012-07-18T18:29:43.743 に答える
0

2 つ (場合によっては 3 つ) を超える引数を持つ関数は、常に配列を渡すため、次のようになります。

public function __construct(array $options = array()) {

    // Figure out which ones you truly need
    if ((!isset($options['arg1'])) || (mb_strlen($options['arg1']) < 1)) {
        throw new Exception(sprintf('Invalid $options[arg1]: %s', serialize($options)));
    }

    // Optional would look like
    $this->member2 = (isset($options['arg1'])) && ((int) $options['arg2'] > 0)) ? $options['arg2'] : null;

    // Localize required params (already validated above)
    $this->member1 = $options['arg1'];
}

オプションの配列を渡すと、関数のシグネチャを変更することなく、将来の拡張が可能になります。ただし、関数が配列のすべての要素をローカライズして、アクセスが警告/エラーをスローしないようにする必要があるという欠点があります (要素が配列にない場合)。

この場合、ファクトリ ソリューションは適切な選択ではありません。ファクトリに値を渡してオブジェクトを正しい値で初期化するという問題が残っているためです。

于 2012-07-17T00:24:34.273 に答える
0

「コンストラクターの引数が多すぎる」に対する標準的な解決策は、ビルダー パターンです。コントローラー クラス自体は引き続き長いコンストラクターを持ちますが、クライアントはビルダーでセッターを使用できます。これにより、後で長いコンストラクターが呼び出されます。

コントローラー オブジェクトを 1 つか 2 つの場所でしか構築しない場合、ビルダーを作成するのに苦労する価値さえありません。その場合は、現在のコードをそのまま使用してください。

于 2012-07-18T09:25:44.660 に答える
0

セット数のパラメーターのコンストラクターを使用する代わりに、セッターを呼び出すファクトリを使用してオブジェクトを作成すると、はるかに柔軟になります。ビルダーとファクトリーのパターンを確認してください。

完全に構築されていないオブジェクトにアクセスするために例外をスローするのは良いことです!

于 2012-07-17T00:10:30.043 に答える