这个MVC控制器代码是否需要重构？

我正在为一个MVC应用程序（Kohana/PHP）编写一个CSV/Excel - > MySQL导入管理器。这个MVC控制器代码是否需要重构？

我有一个控制器名为“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模式编程经验管理这个抽象，应该重构上面的助手类回到控制器或不？

来源

2011-01-19 Edward Tanguay

控制器可以对视图直接操作，但不能呈现自己（它的观点责任）。 “ImportMnager”不是控制器，它是一个View。只要看看你的代码。 – 2011-01-19 08:15:33

`ImportManager`不是渲染自己，而是简单地准备数据的集合（工作表导入），然后视图显示它的意图。所以`ImportManager`实际上是一个控制器，它准备数据并决定哪个视图显示那个数据。 – 2011-01-19 08:23:21

我同意你的说法。

你的ImportController做了什么控制器是做什么的。它生成供用户查看和操作的文件列表，然后将该列表传递给View以显示。我假设你有一个process动作或类似的处理请求，当用户选择一个文件时，这个文件然后被传递给有问题的帮助器。

帮助者是一个完美的抽象例子，在使用和存在方面完全合理。它无论如何都不与控制器耦合，也不需要。 Helper可以很容易地用于Controller不存在的其他场合，例如CRON任务，一个公共API，用户可以通过编程方式调用，而不需要您的ImportController。

你对这个球的权利。把它贴在他们身上！

来源

2011-01-19 10:22:43

控制器有两种方法：使其变薄或变厚。当我开始使用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等加处理的新类型

来源

2011-01-19 10:33:41

这个MVC控制器代码是否需要重构？

回答

相关问题