2013-04-12 1 views
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, где мы проверяем, 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) 
{ 
    ... 

    ... 
} 

Это хороший подход? Если нет, то почему?

ответ

2

Я думаю, что вы можете сохранить свою нынешнюю структуру. Методы действительно должны быть разработаны, чтобы сделать одно. Ваш метод _validateXml проверяет, существует ли схема, которая звучит как метод, который должен выполнять метод проверки XML. Проверка схемы не обязательно является чем-то, что должен сделать метод преобразования, поскольку схема является необязательной.

+1

Я согласен с этим. Кроме того, если '_validateXml' вызывается из других функций, вам также придется выполнить проверку валидации во всех этих случаях. Не очень СУХОЙ. –

2

Если вы думаете об ответственности каждого метода, это должно быть проще понять.

Функция преобразования выполняет преобразование и делегирует валидацию другому методу. Transform только знает, что ему нужно передать имя файла функции проверки подлинности - на самом деле все равно, что делает с ним функция validator.

Таким образом, на мой взгляд, имеет смысл хранить проверки, связанные с файлами, и т. Д. В функции проверки. Или, если вы хотите повторить рефакторинг, возможно, разделить загрузку файлов и проверку файлов на другой метод или создать совершенно отдельный класс для проверки.