1

私は 1 つのクラスのリファクタリングで忙しく、2 つのメソッドをリファクタリングする方法を疑問に思っています。どうぞ：

public function transform($transformXml, $importXml, $xsdScheme = '')
{
    ...
    if (!empty($xsdScheme)) {
        $this->_validateXml($exportDoc, $xsdScheme);
    }
    ...
}

protected function _validateXml(DOMDocument $xml, $xsdScheme)
{
    ...
    if (!file_exists($xsdScheme)) {
        throw new Exception('XSD file was not found in ' . $xsdScheme);
    }
    ...
}

$xsdSchemeメソッドのパラメーターtransformはオプションです。空の場合は xsd 検証を適用しません。その後、 method を呼び出します。_validateXmlここで if をチェックしますfile_exists。この検証は 2 つの部分に分かれていますが、私は好きではありません。だから、私はこのようなものを書きます：

public function transform($transformXml, $importXml, $xsdScheme = '')
{
    ...
    if (!empty($xsdScheme)) {
        if (!file_exists($xsdScheme)) {
            throw new Exception('XSD file was not found in ' . $xsdScheme);
        }

        $this->_validateXml($exportDoc, $xsdScheme);
    }
    ...
}

protected function _validateXml(DOMDocument $xml, $xsdScheme)
{
    ...

    ...
}

それは良いアプローチですか？いいえの場合、なぜですか?

4

2 に答える 2

2

各メソッドの責任について考えれば、これは理解しやすいはずです。

transform 関数は変換を行い、検証を他のメソッドに委譲します。Transform は、ファイルの名前をバリデータ関数に渡す必要があることだけを知っています。バリデータ関数がそれをどう処理するかはあまり気にしません。

したがって、私の意見では、ファイル関連のチェックなどを検証関数に保持することは理にかなっています。または、リファクタリングをさらに進めたい場合は、ファイルの読み込みとファイルのチェックを別のメソッドに分割するか、検証を実行するための完全に別のクラスを作成します。

于 2013-04-12T16:43:47.137 に答える
2

現在の構造を維持したいと思うかもしれません。メソッドは、実際には 1 つのことを行うように設計する必要があります。_validateXml メソッドは、スキームが存在するかどうかを確認します。これは、XML 検証メソッドが行うべきことのように思えます。スキームはオプションであるため、スキームのチェックは必ずしも変換メソッドで実行する必要があるとは限りません。

于 2013-04-12T16:42:33.390 に答える