Должен ли быть реорганизован этот код контроллера MVC или нет?

Question

Я пишу CSV/Excel -> менеджер импорта MySQL для приложения MVC (Kohana/PHP).

У меня есть контроллера под названием «ImportManager», который имеет действие под названием «Индекс» (по умолчанию), который отображает в сетке всех действительных .csv и .xls файлов, которые находятся в определенной директории и готовы для импорта. Затем пользователь может выбрать файлы, которые он хочет импортировать.

Однако, поскольку .csv файлов импорт в таблицу базы данных один и .xls файлов импорт в таблиц базы данных множественного, мне нужно, чтобы справиться с этой абстракции. Поэтому я создал вспомогательный класс под названием SmartImportFile, к которым я посылаю каждый файл будет его .csv или .xls, а затем я получаю то задаю этот «умный» объект, чтобы добавить листы из этого файла (будь то один или много) к моему коллекция. Вот мой метод действия в PHP код:

public function action_index() 
{ 
    $view = new View('backend/application/importmanager'); 

    $smart_worksheets = array(); 
    $raw_files = glob('/data/import/*.*'); 
    if (count($raw_files) > 0) 
    { 
     foreach ($raw_files as $raw_file) 
     { 
      $smart_import_file = new Backend_Application_Smartimportfile($raw_file); 
      $smart_worksheets = $smart_import_file->add_smart_worksheets_to($smart_worksheets); 
     } 
    } 
    $view->set('smart_worksheets', $smart_worksheets); 

    $this->request->response = $view; 
} 

Класс SmartImportFile выглядит следующим образом:

class Backend_Application_Smartimportfile 
{ 
    protected $file_name; 
    protected $file_extension; 
    protected $file_size; 
    protected $when_file_copied; 
    protected $file_name_without_extension; 
    protected $path_info; 
    protected $current_smart_worksheet = array(); 

    protected $smart_worksheets = array(); 

    public function __construct($file_name) 
    { 
     $this->file_name = $file_name; 
     $this->file_name_without_extension = current(explode('.', basename($this->file_name))); 

     $this->path_info = pathinfo($this->file_name); 
     $this->when_file_copied = date('Y-m-d H:i:s', filectime($this->file_name)); 
     $this->file_extension = strtolower($this->path_info['extension']); 
     $this->file_extension = strtolower(pathinfo($this->file_name, PATHINFO_EXTENSION)); 
     if(in_array($this->file_extension, array('csv','xls','xlsx'))) 
     { 
      $this->current_smart_worksheet = array(); 
      $this->process_file(); 
     } 
    } 

    private function process_file() 
    { 
     $this->file_size = filesize($this->file_name); 
     if(in_array($this->file_extension, array('xls','xlsx'))) 
     { 
      if($this->file_size < 4000000) 
      { 
       $this->process_all_worksheets_of_excel_file(); 
      } 
     } 
     else if($this->file_extension == 'csv') 
     { 
      $this->process_csv_file(); 
     } 

    } 

    private function process_all_worksheets_of_excel_file() 
    { 
     $worksheet_names = Import_Driver_Excel::get_worksheet_names_as_array($this->file_name); 
     if (count($worksheet_names) > 0) 
     { 
      foreach ($worksheet_names as $worksheet_name) 
      { 
       $this->current_smart_worksheet['name'] = basename($this->file_name).' ('.$worksheet_name.')'; 
       $this->current_smart_worksheet['kind'] = strtoupper($this->file_extension); 
       $this->current_smart_worksheet['file_size'] = $this->file_size; 
       $this->current_smart_worksheet['when_file_copied'] = $this->when_file_copied; 
       $this->current_smart_worksheet['table_name'] = $this->file_name_without_extension.'__'.$worksheet_name; 
       $this->assign_database_table_fields(); 
       $this->smart_worksheets[] = $this->current_smart_worksheet; 
      } 
     } 
    } 

    private function process_csv_file() 
    { 
     $this->current_smart_worksheet['name'] = basename($this->file_name); 
     $this->current_smart_worksheet['kind'] = strtoupper($this->file_extension); 
     $this->current_smart_worksheet['file_size'] = $this->file_size; 
     $this->current_smart_worksheet['when_file_copied'] = $this->when_file_copied; 

     $this->current_smart_worksheet['table_name'] = $this->file_name_without_extension; 
     $this->assign_database_table_fields(); 


     $this->smart_worksheets[] = $this->current_smart_worksheet; 
    } 

    private function assign_database_table_fields() 
    { 
     $db = Database::instance('import_excel'); 
     $sql = "SHOW TABLE STATUS WHERE name = '".$this->current_smart_worksheet['table_name']."'"; 
     $result = $db->query(Database::SELECT, $sql, FALSE)->as_array(); 
     if(count($result)) 
     { 
      $when_table_created = $result[0]['Create_time']; 
      $when_file_copied_as_date = strtotime($this->when_file_copied); 
      $when_table_created_as_date = strtotime($when_table_created); 
      if($when_file_copied_as_date > $when_table_created_as_date) 
      { 
       $this->current_smart_worksheet['status'] = 'backend.application.import.status.needtoreimport'; 
      } 
      else 
      { 
       $this->current_smart_worksheet['status'] = 'backend.application.import.status.isuptodate'; 
      } 
      $this->current_smart_worksheet['when_table_created'] = $when_table_created; 
     } 
     else 
     { 
      $this->current_smart_worksheet['when_table_created'] = 'backend.application.import.status.tabledoesnotexist'; 
      $this->current_smart_worksheet['status'] = 'backend.application.import.status.needtoimport'; 
     } 
    } 

    public function add_smart_worksheets_to(Array $smart_worksheets = array()) 
    { 
     return array_merge($smart_worksheets, $this->get_smart_worksheets()); 
    } 

    public function get_smart_worksheets() 
    { 
     if (! is_array($this->smart_worksheets)) 
     { 
      return array(); 
     } 

     return $this->smart_worksheets; 
    } 

} 

В обзоре в кода мне сказали, что это могло бы быть лучше не иметь помощника класс, но сохранить основную часть кода в самом методе действия контроллера. Аргументация заключалась в том, что вы должны иметь возможность взглянуть на код в действии контроллера и посмотреть, что он делает, вместо того, чтобы он вызывал внешние вспомогательные классы вне себя. Я не согласен. Моя аргументация:

• вы должны создать вспомогательный класс в любое время он делает код более четкими, так как в этом случае, он абстрагирует тот факт, что некоторые файлы имеют один листа или много листов в них , и позволяет легко продлить будущее, если, скажем, мы также хотим импортировать из sqlite файлы или даже каталоги с файлами в них, эта абстракция этого класса сможет справиться с этим.
• Перемещение основной части кода из этого вспомогательного класса обратно в контроллер заставит меня создать внутренние переменные в контроллере, которые имеют смысл для этого конкретного действия, но могут или не могут иметь смысл для других методов действий в пределах контроллер.
• если я программировал это C# Я хотел бы сделать этот помощник класс а вложенного класс, который в буквальном смысле быть внутренней структурой данные, которые внутри и доступно только для класса контроллера, но так как PHP не допускает вложенных классов, мне нужно вызвать класс «вне» контроллер для управления этой абстракции таким образом, что делает код ясным и читаемым

Основываясь на вашем опыте программирования в паттерне MVC, следует переопределить класс вспомогательного класса вернуться в контроллер или нет?

Answer 1

Я согласен с вами, Эдвард.

Ваш ImportController выполняет то, что должен выполнять контроллер. Он генерирует список файлов для просмотра и действия пользователя, затем он передает этот список в представление для его отображения. Я предполагаю, что у вас есть действие process или подобное, которое обрабатывает запрос, когда пользователь выбрал файл, этот файл затем передается соответствующему помощнику.

Помощник - прекрасный пример абстракции и полностью оправдан в его использовании и существовании. Во всяком случае, он не связан с контроллером и не нужен. Помощник может быть легко использован в других сценариях, где Контроллер отсутствует, например задача CRON, открытый API, который пользователи могут вызывать программно без вашего ImportController.

Ваше право на мяч с этим. Прикрепи его к ним!

Answer 2

Существует два подхода к контроллерам: сделать его тонким или толстым. Когда я начал свое приключение с MVC, я ошибся в создании толстых контроллеров - теперь я предпочитаю сделать его как можно более тонким. На мой взгляд, ваше решение хорошее.

Вот как я переработан код еще дальше:

class Backend_Application_SmartImport { 

    public function __construct($raw_files) { 
    } 

    public function process() {  
     foreach ($raw_files as $raw_file) { 
      // (...) 
      $oSmartImportFileInstance = $this->getSmartImportFileInstance($smart_import_file_extension); 
     } 
    } 

    protected function getSmartImportFileInstance($smart_import_file_extension) { 
     switch ($smart_import_file_extension) { 
      case 'xml': 
       return new Backend_Application_SmartImportFileXml(); 
      // (...) 
     } 
    } 
} 

abstract class Backend_Application_SmartImportFile { 
    // common methods for importing from xml or cvs 
    abstract function process(); 
} 

class Backend_Application_SmartImportFileCVS extends Backend_Application_SmartImportFile { 
    // methods specified for cvs importing 
} 

class Backend_Application_SmartImportFileXls extends Backend_Application_SmartImportFile { 
    // methods specified for xls importing 
} 

Идея заключается в том, чтобы иметь два класса, отвечающие за обработку XML и CVS, наследующих от базового класса. Основной класс использует специальный метод для определения способа обработки данных (Strategy Pattern). Контроллер просто передал список файлов экземпляру класса Backend_Application_SmartImport и передает результат метода процесса в представление.

Преимущество моего решения заключается в том, что код более Развязанного и вы можете легко и в чистом виде добавлять новые типы обработки, как XML, PDF и т.д.