2017-04-05 10 views
20

あまりにも多くの作業を行っているコンストラクタについてはan articleと読んでいます。 一つの段落は、依存関係が反転する傾向にあるオブジェクト指向スタイルでは、有効なオブジェクト状態を定義するものは何ですか?

を読み込み、コンストラクタは異なる、よりスパルタンな役割を持っています。その唯一の仕事は、オブジェクトが基本不変条件を満たす状態にオブジェクトが初期化されることを確認することです(つまり、オブジェクトインスタンスが有効な状態で始まり、それ以上のものでないことを確認します)。

ここでは、クラスの基本的な例を示します。クラスの作成時に、HTMLで渡されます。このHTMLは、クラスプロパティを設定するために解析する必要があります。

OrderHtmlParser 
{ 
    protected $html; 

    protected $orderNumber; 

    public function __construct($html) 
    { 
     $this->html = $html; 
    } 

    public function parse() 
    { 
     $complexLogicResult = $this->doComplexLogic($this->html); 

     $this->orderNumber = $complexLogicResult; 
    } 

    public function getOrderNumber() 
    { 
     return $this->orderNumber; 
    } 

    protected function doComplexLogic($html) 
    { 
     // ... 

     return $complexLogicResult; 
    } 
} 

私はコンストラクタが、これはひどい習慣で、上記の記事とthis article状態の両方として任意のロジックをやっていけないので、私はparse機能を使用

$orderparser = new OrderHtmlParser($html); 
$orderparser->parse() 
$orderparser->getOrderNumber(); 

を使用してそれを呼んでいます。私はparseメソッドを使用しない場合

public function __construct($html) 
{ 
    $this->html = $html; 
    $this->parse(); // bad 
} 

しかし、その後、私のすべてのプロパティ(この例では一つだけ)はnullを返します。

これは「無効な状態」のオブジェクトとして知られていますか?私の解析方法は、それがときにコンストラクタが、そのメソッドを呼び出したときにのみである場合も、他の物品(イムわからないもののにより悪いとみなされる変装initialise関数であるよう

はまた、それがややを感じています手動で呼び出されるか、その両方)。それにもかかわらず、初期化メソッドは、プロパティを設定する前にいくつかの複雑なロジックを実行しています。ゲッターが確実に呼び出される前に発生する必要があります。

私はこれらの記事を誤解しているか、これらの記事で私がこの単純なクラスの全体的な実装が正しくないと思うようになってしまっています。

+4

'$ html'をコンストラクタに渡すのではなく、' parse'メソッドに渡すことを検討してください。次に、他のメソッド(nullを返す)の振る舞いがより理にかなっています。あなたは引数をコンストラクタに渡すこともできますが(オプション)、コンストラクタはオブジェクトの* parse *メソッドを呼び出す必要があります。どうやら、オプションの振る舞いは記事に書かれているビジョンに合わないが、それは私には受け入れられるようだ。 – trincot

+0

それは私を助けてくれてありがとう。 –

+0

@myol - 私は自分の返信を更新し、一般的なコンストラクタとコンストラクタの例外についての考慮事項を追加しました。 –

答えて

6

一般的にコンストラクタで作業を行うのはコードの匂いですが、練習の背後にある理由はベストプラクティスに関する意見よりもプログラミング言語と関係があります。バグを導入する真のエッジケースがあります。

いくつかの言語では、派生クラスのコンストラクタはボトムアップから実行され、他の言語はトップダウンから実行されます。 PHPでは、上から下に呼び出され、parent::__construct()を呼ぶことなくチェーンを停止することさえできます。

これは、基底クラスで未知の状態の期待値を作成し、問題を悪化させるために、PHPではコンストラクタで親を最初か最後に呼び出すことができます。

例:上の例クラスB

class A extends B { 
    public __construct() { 
      $this->foo = "I am changing the state here"; 
      parent::__construct(); // call parent last 
    } 
} 

class A extends B { 
    public __construct() { 
      parent::__construct(); // call parent first 
      $this->foo = "I am changing the state here"; 
    } 
} 

それは異なる順序で呼び出されるコンストラクタだとBはコンストラクタで多くの仕事をやっていたならば、それはプログラマが期待していた状態ではない可能性があります。

どのように問題を解決しますか?

2つのクラスが必要です。 1つはパーサーロジックを含み、もう1つはパーサー結果を含みます。それは注文番号を抽出し、または例をスローに失敗した場合

class OrderHtmlResult { 
     private $number; 
     public __construct($number) { 
      $this->number = $number; 
     } 
     public getOrderNumber() { 
      return $this->number; 
     } 
} 

class OrderHtmlParser { 
     public parse($html) { 
      $complexLogicResult = $this->doComplexLogic($this->html); 
      return new OrderHtmlResult($complexLogicResult); 
     } 
} 

$orderparser = new OrderHtmlParser($html); 
$order = $orderparser->parse($html) 
echo $order->getOrderNumber(); 

は、上記の例では、 parse()メソッドの戻り値 nullを持つことができます。しかし、どちらのクラスも無効な状態に入ることはありません。

このパターンの名前があります。メソッドが状態情報をカプセル化するために結果として別のオブジェクトを生成しますが、その名前が何であるか覚えています。

+0

このパターンは_procedural programming_だと思います。論理からデータを分離することは、主目的がデータと論理を結びつけることであるOOPの逆説である。 – jaco0646

+0

ありがとうございました、私のフォーマットとバリデーターがインターフェイスを使って解析されたオブジェクトを期待できるように、このアプローチをとったのです – myol

3

コンストラクタが「あまりにも多く」行うときに発生する一般的な問題の1つは、幾分密接にリンクされた2つのオブジェクトが互いに参照する必要があるということです(はい、近いリンクは悪い匂いですが、起こります)。

オブジェクトAとオブジェクトBが「有効」になるために互いに参照する必要がある場合、どのように作成するのですか?

答えは通常、コンストラクタが完全に「有効」でないオブジェクトを作成し、他の無効なオブジェクトへの参照を追加してから、ある種のfinalize/initialize/startメソッドを呼び出してオブジェクトを有効にします。

「安全」になりたい場合は、オブジェクトが「有効」になる前に呼び出された場合、初期化されていない例外をスローすることでビジネスメソッドを保護できます。

依存性注入にはこの問題の一般化されたバージョンがあります。注入されたクラスの循環ループがあればどうなりますか? construct/initializeパターンに従えば、一般的なケースも解決されるので、DIは常にそのパターンを使用します。

4

これは「無効な状態」のオブジェクトとして知られていますか?

はい。 parseメソッドが偽装のinitialise機能であることは間違いありません。

初期化の解析を避けるには、が怠惰なになるようにします。最も怠惰な方法は、$orderNumberフィールドを削除して、getOrderNumber()関数内の$htmlフィールドから解析することです。関数が繰り返し呼び出されたり、解析が高価になることが予想される場合は、$orderNumberフィールドをそのままにして、キャッシュとして扱います。 nullの内部がgetOrderNumber()であることを確認し、最初の呼び出しでのみ解析します。


リンクされた記事に関しては、コンストラクタはフィールドの初期化に限定する必要があります。しかし、これらのフィールドがテキストブロックから解析され、クライアントが解析された値のほとんどまたはすべてを利用することが期待される場合、遅延初期化はほとんど価値がありません。 さらに、テキスト解析でIOオブジェクトやドメインオブジェクトが含まれていない場合は、ブラックボックステストを妨げてはいけません。

0

チェック彼は以下のクラスは、状況が変化したときに、それは行動だ変わる状態パターンで

フル助けることができます。

この例では、BookContextクラスは、BookTitleStateStarsで始まるBookTitleStateInterfaceの実装を保持します。 BookTitleStateStarsとBookTitleStateExclaimは、呼び出された回数に応じてBookContext内で互いに置き換えられます。

<?php 

class BookContext { 
    private $book = NULL; 
    private $bookTitleState = NULL; 
    //bookList is not instantiated at construct time 
    public function __construct($book_in) { 
     $this->book = $book_in; 
     $this->setTitleState(new BookTitleStateStars()); 
    } 
    public function getBookTitle() { 
     return $this->bookTitleState->showTitle($this); 
    } 
    public function getBook() { 
     return $this->book; 
    } 
    public function setTitleState($titleState_in) { 
     $this->bookTitleState = $titleState_in; 
    } 
} 

interface BookTitleStateInterface { 
    public function showTitle($context_in); 
} 

class BookTitleStateExclaim implements BookTitleStateInterface { 
    private $titleCount = 0; 
    public function showTitle($context_in) { 
     $title = $context_in->getBook()->getTitle(); 
     $this->titleCount++; 
     $context_in->setTitleState(new BookTitleStateStars()); 
     return Str_replace(' ','!',$title); 
    } 
} 

class BookTitleStateStars implements BookTitleStateInterface { 
    private $titleCount = 0; 
    public function showTitle($context_in) { 
     $title = $context_in->getBook()->getTitle(); 
     $this->titleCount++; 
     if (1 < $this->titleCount) { 
     $context_in->setTitleState(new BookTitleStateExclaim); 
     } 
     return Str_replace(' ','*',$title); 
    } 
} 

class Book { 
    private $author; 
    private $title; 
    function __construct($title_in, $author_in) { 
     $this->author = $author_in; 
     $this->title = $title_in; 
    } 
    function getAuthor() {return $this->author;} 
    function getTitle() {return $this->title;} 
    function getAuthorAndTitle() { 
     return $this->getTitle() . ' by ' . $this->getAuthor(); 
    } 
} 

    writeln('BEGIN TESTING STATE PATTERN'); 
    writeln(''); 

    $book = new Book('PHP for Cats','Larry Truett');; 
    $context = new bookContext($book); 

    writeln('test 1 - show name'); 
    writeln($context->getBookTitle()); 
    writeln(''); 

    writeln('test 2 - show name'); 
    writeln($context->getBookTitle()); 
    writeln(''); 

    writeln('test 3 - show name'); 
    writeln($context->getBookTitle()); 
    writeln(''); 

    writeln('test 4 - show name'); 
    writeln($context->getBookTitle()); 
    writeln(''); 

    writeln('END TESTING STATE PATTERN'); 

    function writeln($line_in) { 
    echo $line_in."<br/>"; 
    } 

?> 

出力

BEGIN TESTING STATE PATTERN 

test 1 - show name 
PHP*for*Cats 

test 2 - show name 
PHP*for*Cats 

test 3 - show name 
PHP!for!Cats 

test 4 - show name 
PHP*for*Cats 

END TESTING STATE PATTERN 

参照私は完全に@trincotからのコメントに同意するreference

1

から:

あなたはコンストラクタを持つパーサを作成し、何もありませんhtmlを渡す必要があります。

もう1つの入力でもう1つの時間をパーサーオブジェクトで使用したいことがあります。

クリーンなコンストラクタを使用するには、reset()関数を使用します。この関数はBeginningでも呼び出され、オブジェクトの初期状態をリセットします。

例:このように

class OrderHtmlParser 
{ 
    protected $html; 
    protected $orderNumber; 

    public function __construct() 
    { 
    $this->reset(); 
    } 

    public function reset() 
    { 
    $this->html = null; 
    $this->orderNumber = null; 
    } 
    /** 
    * Parse the given Context and return the result 
    */ 
    public function parse($html) 
    { 
    // Store the Input for whatever 
    $this->html = $html; 
    // Parse 
    $complexLogicResult = $this->doComplexLogic($this->html); 
    // Store the Result for whatever 
    $this->orderNumber = $complexLogicResult; 
    // return the Result 
    return $this->orderNumber; 
    } 

    public function getOrderNumber(){} 
    protected function doComplexLogic($html){} 
} 

、解析オブジェクトは、行うことになっているものを、行うことができます。

解析など、多くの場合、あなたが望むよう:

質問への対処
$parser = new OrderHtmlParser(); 
$result1 = $parser->parse($html1); 
$parser->reset(); 
$result2 = $parser->parse($html2); 
+0

ありがとう、リセット機能のアイデアは今では分かりやすいですが、私は間違いなくこのことを検討します – myol

3

タイトルの中で私はオブジェクトが何の問題もなく作業を実行することができるとき、オブジェクトを常に有効な状態にあるものとして見てきました。つまり期待どおりに動作します。

私に飛び出したものは、コンストラクタロジックが多くのオブジェクトを作成していたことでした。私は7を数えました。これらのオブジェクトはすべて、参照されているクラス(ActiveProduct)直接コンストラクタは、他のオブジェクトのコンストラクタにこのポインタを渡した:

VirtualCalculator = new ProgramCalculator(this, true); 
    DFS = new DFSCalibration(this); 

は、この場合ActiveProductはまだその初期化を完了していない、まだProgramCalculatorとDFSCalibrationはメソッドとプロパティを経由して戻ってActiveProductにコールとのすべての種類を引き起こす可能性がありますいたずらしているので、コードは非常に疑わしいです。 一般に、OOPでは、オブジェクトをコンストラクタに渡して、コンストラクタでインスタンス化しないことが必要です。また、Dependency Inversion Principleを採用し、dependency injectionを可能にするオブジェクトをコンストラクタに渡すときに、インタフェースまたは抽象/純粋仮想クラスを使用したいとします。

OrderHtmlParserクラスのケースでは、問題の複雑なロジックがOrderHtmlParserクラスの外に出ていないように見えるため、これは問題ではないようです。 doComplexLogic関数が保護されていると定義されている理由について興味があり、継承するクラスで呼び出すことができます。

これは、Parseメソッドを静的にし、それを使ってOrderHtmlParserクラスのインスタンスを構築し、コンストラクタをprivateにすることで、呼び出し側がParseメソッドを呼び出して例:

OrderHtmlParser 
{ 
    protected $html; 

    protected $orderNumber; 

    private function __construct() 
    { 

    } 

    public static function parse($html) 
    { 
     $instance = new OrderHtmlParser(); 

     $instance->html = $html; 

     $complexLogicResult = $instance->doComplexLogic($this->html); 

     $instance->orderNumber = $complexLogicResult; 

     return $instance; 
    } 

    public function getOrderNumber() 
    { 
     return $this->orderNumber; 
    } 

    protected function doComplexLogic($html) 
    { 
     // ... 

     return $complexLogicResult; 
    } 
} 
1

ありがとうございました!

大規模なデータを後で処理するために、大量のデータをコンストラクタに渡してオブジェクト内に格納するだけのエラーが発生しやすい設計です。

は私が(太字マークは私です)もう一度あなたの美しい引用を引用してみましょう:依存関係が反転する傾向にあるオブジェクト指向スタイルでは、

、コンストラクタは異なる、よりスパルタンな役割を持っています。 その唯一の仕事は、オブジェクトが基本不変式を満たす状態にオブジェクトが初期化されることを確認することです(つまり、オブジェクトインスタンスが有効な状態で開始され、それ以上のものがないことを確認します)。

あなたの例のパーサークラスの設計は、以下の引用符で言及されているような "初期化データ"だけでなく、実際のデータである入力データを受け取りますが、実際に処理しないので面倒ですデータ。

非常に古いプログラミングコースでは、1980年代に、プログラムに入出力があると言われました。

プログラム入力の$ htmlを考えてください。

コンストラクタは、プログラム入力を受け入れるはずではありません。彼らは、文字セット名のような設定、初期化データ、または後で提供されないかもしれない他の設定パラメータを受け入れることになっています。大規模なデータを受け入れる場合は、時々例外をスローする必要があり、コンストラクターの例外は非常に悪いスタイルです。コンストラクタの例外は、すべてのコストをかけて回避する必要があります。たとえば、ファイル名をコンストラクタに渡すこともできますが、コンストラクタ内でファイルを開くなどしないでください。

あなたのクラスを少し修正してください。

enum ParserState (undefined, ready, result_available, error); 


OrderHtmlParser 
{ 

    protected $orderNumber; 
    protected $defaultEncoding; 
    protected ParserState $state; 

    public function __construct($orderNumber, $defaultEncoding default "utf-8") 
    { 
     $this->orderNumber = $orderNumber; 
     $this->defaultEncoding = $defaultEncoding; 
     $this->state = ParserState::ready; 
    } 

    public function feed_data($data) 
    { 
     if ($this->state != ParserState::ready) raise Exception("You can only feed the data to the parser when it is ready"); 

     // accumulate the data and parse it until we get enough information to make the result available 

     if we have enough result, let $state = ParserState::resultAvailable; 
    } 

    public function ParserState getState() 
    { 
     return $this->state 
    } 

    public function getOrderNumber() 
    { 
     return $this->orderNumber; 
    } 

    protected function getResult($html) 
    { 
     if ($this->state != ParserState::resultAvailable) raise Exception("You should wait until the result is available"); 

     // accumulate the data and parse it until we get enough information to make the result available 
    } 
} 

明白なデザインになるようにクラスを設計する場合、人々はどのメソッドも呼び出すことを忘れることはありません。論理に反して、コンストラクタはデータを取ったが何もしなかったため、明らかではない特別な機能が必要だったため、オリジナルの質問のデザインには欠陥がありました。あなたがデザインをシンプルで明白にするなら、あなたは州も必要としません。状態は、例えば、TCP/IPソケットからHTMLを非同期に読み取ってこのデータをパーサに送る場合など、結果が得られるまで、長い間データを蓄積するクラスに対してのみ必要です。

$orderparser = new OrderHtmlParser($orderNumber, "Windows-1251"); 
repeat 
    $data = getMoreDataFromSocket(); 
    $orderparser->feed_data($data); 
until $orderparser->getState()==ParserState::resultAvailable; 
$orderparser->getResult(); 

オブジェクトの状態に関する最初の質問については、データを受け取って処理するメソッドがあるときにコンストラクタが初期化データを取得するようにクラスを設計すると、データを格納し、呼び出しを忘れた可能性のあるデータを解析する別個の関数は存在しません。必要です。データを順番に収集または供給する長寿命オブジェクトの状態が依然として必要な場合は、上記の例のような列挙型を使用できます。私の例は抽象的な言語であり、特定のプログラミング言語ではありません。

+1

ありがとう、あなたの答えは実際に入力と出力を説明することによってアプローチの理由を説明しました。あなたは私にオブジェクトのやりとりについて完全に(微妙ではありますが)異なる考え方を与えました。私は週末に忙しかったので、自動的に奨励金を割り当てたので、私はあなたにこの答えのために与えるもう一つを追加しました。 – myol

+0

@myol - 本当にありがとう! –

関連する問題