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 验证。在它之后我们调用方法_validateXml，我们正在检查 if file_exists。此验证分为两部分，我不喜欢它，我更喜欢它在一个地方。所以，我会写这样的东西：

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

我想你可能想保持你目前的结构。方法真的应该被设计成做一件事。您的 _validateXml 方法检查该方案是否存在，这听起来像是 XML 验证方法应该做的事情。检查方案不一定是转换方法需要做的事情，因为方案是可选的。

于 2013-04-12T16:42:33.390 回答