這個MVC控制器代碼是否需要重構？

Question

我正在爲一個MVC應用程序（Kohana/PHP）編寫一個CSV/Excel - > MySQL導入管理器。

我有一個控制器名爲「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做了什麼控制器是做什麼的。它生成供用戶查看和操作的文件列表，然後將該列表傳遞給View以顯示。我假設你有一個process動作或類似的處理請求，當用戶選擇一個文件時，這個文件然後被傳遞給有問題的幫助器。

幫助者是一個完美的抽象例子，在使用和存在方面完全合理。它無論如何都不與控制器耦合，也不需要。 Helper可以很容易地用於Controller不存在的其他場合，例如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等加處理的新類型