Рефакторинг Laravel 4 App

Question

Я нахожусь в процессе рефакторинга моего приложения Laravel 4, и я хотел бы получить совет по моему подходу и лучшей практике в будущем. Я стараюсь быть как можно сухим, а также следовать принципам SOLID.

У меня есть метод обновления, который обновляет делегаты в моем приложении. Существует много разных отношений между делегатами и событиями, и мне нужно проверить, выполняется ли несколько условий при обновлении, чтобы выполнить правильное действие/функцию.

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

public function update($id) 
{ 
    $delegate = $this->delegate->findById($id); 
    $input = Input::all(); 

    if ($delegate->isFloater()) 
    { 
     if (empty($input['new_event_id'])) 
     { 
      $message = (object) array(
       'title'   => 'Oops!', 
       'content'  => 'You did not select an event to move this floating delegate to.', 
       'alert_type' => 'error' 
      ); 

      return Redirect::back()->withInput()->with('message', $message); 
     } 

     $newEvent = $this->event->findById($input['new_event_id']); 

     if (! $newEvent->hasCapacity()) 
     { 
      $message = (object) array(
       'title'   => 'Oops!', 
       'content'  => 'The event you are trying to move this floating delegate to has no availability.', 
       'alert_type' => 'error' 
      ); 

      return Redirect::back()->withInput()->with('message', $message); 
     } 

     if (! $newEvent->hasResitCapacity($input)) 
     { 
      $message = (object) array(
       'title'   => 'Oops!', 
       'content'  => 'The event you are trying to move this floating delegate to has no resit spaces left.', 
       'alert_type' => 'error' 
      ); 

      return Redirect::back()->withInput()->with('message', $message); 
     } 

     if (! $newEvent->hasExamCapacity($input)) 
     { 
      $message = (object) array(
       'title'   => 'Oops!', 
       'content'  => 'The event you are trying to move this floating delegate to has no exam spaces left.', 
       'alert_type' => 'error' 
      ); 

      return Redirect::back()->withInput()->with('message', $message); 
     } 

     $this->delegate->moveFloater($id, $input, $newEvent); 

     $message = (object) array(
      'title'   => 'Excellent!', 
      'content'  => 'This floating delegate was successfully moved.', 
      'alert_type' => 'success' 
     ); 

     return Redirect::to('admin/events')->with('message', $message); 
    } 
} 

Это, конечно, гораздо лучше, чем я был раньше, и я абстрагирование код для различных модельных методов, таких как hasCapacity() и isFloater() и т.д., но уже метод является большим, и я повторяю код относится к сообщениям и перенаправлениям.

На данный момент я не уверен, как лучше всего подойти к этому, и я ищу несколько советов о том, как действовать дальше и очищать его дальше.

Любая помощь будет высоко оценена. Заранее спасибо.

Answer 1

Прежде всего, может ли перемещаться делегат, это логика бизнеса/домена и не принадлежит контроллеру. Переместите всю логику, связанную с этим, в класс репозитория/службы, который возвращает false, если что-то пойдет не так, а затем используйте метод getError для получения сообщения об ошибке.

Во-вторых, конструкция флэш-переменной 'message' может быть абстрагирована в защищенный метод, достигая DRY.

В-третьих, «обновление» для описания действия контроллера, которое явно направлено на перемещение делегата из одного события в другое, вводит в заблуждение, поэтому я переименую его просто «переместить».

public function move($id) 
{ 
    $delegate = $this->delegate->findById($id); 

    if (!$delegate) { 
     return $this->notFound(); 
     // or something like this... 
     App::abort(404); 
    } 

    $input = Input::all(); 

    if (!$this->delegate->move($delegate, $input)) { 
     $error = $this->delegate->getError(); 
     $message = $this->wrapMessage('error', 'Oops!', $error) 
     return Redirect::back()->withInput()->with('message', $message); 
    } 

    $message = $this->wrapMessage('success', 'Excellent!', 'This floating delegate was successfully moved.'); 
    return Redirect::to('admin/events')->with('message', $message); 
} 

protected function wrapMessage($type, $title, $content) 
{ 
    return (object) array(
     'title'  => $title, 
     'content' => $content, 
     'alert_type' => $type 
    ); 
}