Laravel 5 - Clean code, where to keep business logic (controller example)

Question

Below example of 'store' method of my controller Admin/MoviesController. It already seems quite big, and 'update' method will be even bigger.

The algoritm is:

1. Validate request data in CreateMovieRequest and create new movie with all fillable fields.
2. Upload poster
3. Fill and save all important, but not required fields (Meta title, Meta Description..)
4. Then 4 blocks of code with parsing and attaching to movie of Genres, Actors, Directors, Countries.
5. Request of IMDB's rating using third-party API

My questions:

• Should I just move all this code to Model and divide it into smaller methods like: removeGenres($id), addGenres(Request $request), ...

• Are there some best practices? I'm talking not about MVC, but Laravel's features. At the moment to keep some logic behind the scene I'm using only Request for validation.

  public function store(CreateMovieRequest $request) {
  
  $movie = Movies::create($request->except('poster'));
  
  /* Uploading poster */
  if ($request->hasFile('poster')) {
      $poster = \Image::make($request->file('poster'));
  
      $poster->fit(250, 360, function ($constraint) {
          $constraint->upsize();
      });
  
      $path = storage_path() . '/images/movies/'.$movie->id.'/';
  
      if(! \File::exists($path)) {
          \File::makeDirectory($path);
      }
  
      $filename = time() . '.' . $request->file('poster')->getClientOriginalExtension();
      $poster->save($path . $filename);
      $movie->poster = $filename;
  }
  
  /* If 'Meta Title' is empty, then fill it with the name of the movie */
  if ( empty($movie->seo_title) ) {
      $movie->seo_title = $movie->title;
  }
  
  /* If 'Meta Description' is empty, then fill it with the description of the movie */
  if ( empty($movie->seo_description) ) {
      $movie->seo_description = $movie->description;
  }
  
  // Apply all changes
  $movie->save();
  
  /* Parsing comma separated string of genres
   * and attaching them to movie */
  if (!empty($request->input('genres'))) {
  
      $genres = explode(',', $request->input('genres'));
  
      foreach($genres as $item) {
          $name = mb_strtolower(trim($item), 'UTF-8');
  
          $genre = Genre::where('name', $name)->first();
  
          /* If such genre doesn't exists in 'genres' table
           * then we create a new one */
          if ( empty($genre) ) {
              $genre = new Genre();
              $genre->fill(['name' => $name])->save();
          }
  
          $movie->genres()->attach($genre->id);
      }
  }
  
  /* Parsing comma separated string of countries
   * and attaching them to movie */
  if (!empty($request->input('countries'))) {
      $countries = explode(',', $request->input('countries'));
      foreach($countries as $item) {
          $name = mb_strtolower(trim($item), 'UTF-8');
  
          $country = Country::where('name', $name)->first();
  
          if ( empty($country) ) {
              $country = new Country();
              $country->fill(['name' => $name])->save();
          }
  
          $movie->countries()->attach($country->id);
      }
  }
  
  /* Parsing comma separated string of directors
   * and attaching them to movie */
  if (!empty($request->input('directors'))) {
      $directors = explode(',', $request->input('directors'));
      foreach($directors as $item) {
          $name = mb_strtolower(trim($item), 'UTF-8');
  
          // Actors and Directors stored in the same table 'actors'
          $director = Actor::where('fullname', trim($name))->first();
  
          if ( empty($director) ) {
              $director = new Actor();
              $director->fill(['fullname' => $name])->save();
          }
          // Save this relation to 'movie_director' table
          $movie->directors()->attach($director->id);
      }
  }
  
  /* Parsing comma separated string of actors
   * and attaching them to movie */
  if (!empty($request->input('actors'))) {
  
      $actors = explode(',', $request->input('actors'));
      foreach($actors as $item) {
          $name = mb_strtolower(trim($item), 'UTF-8');
  
          $actor = Actor::where('fullname', $name)->first();
  
          if ( empty($actor) ) {
              $actor = new Actor();
              $actor->fill(['fullname' => $name])->save();
          }
  
          // Save this relation to 'movie_actor' table
          $movie->actors()->attach($actor->id);
      }
  }
  
  // Updating IMDB and Kinopoisk ratings
  if (!empty($movie->kinopoisk_id)) {
      $content = Curl::get('http://rating.kinopoisk.ru/'.$movie->kinopoisk_id.'.xml');
  
      $xml = new \SimpleXMLElement($content[0]->getContent());
  
      $movie->rating_kinopoisk = (double) $xml->kp_rating;
      $movie->rating_imdb = (double) $xml->imdb_rating;
      $movie->num_votes_kinopoisk = (int) $xml->kp_rating['num_vote'];
      $movie->num_votes_imdb = (int) $xml->imdb_rating['num_vote'];
  
      $movie->save();
  }
  
  return redirect('/admin/movies');
  }
  

Answer 1

You need to think on how you could re-utilize the code if you need to use it in another classes or project modules. For starting, you could do something like this:

Movie model, can improved in order to:

• Manage the way on how the attributes are setted
• Create nice functions in functions include/manage the data of relationships

Take a look how the Movie implements the functions:

class Movie{

    public function __construct(){

        //If 'Meta Title' is empty, then fill it with the name of the movie
        $this->seo_title = empty($movie->seo_title)
            ? $movie->title 
            : $otherValue;

        //If 'Meta Description' is empty, 
        //then fill it with the description of the movie
        $movie->seo_description = empty($movie->seo_description)
            ? $movie->description 
            : $anotherValue;

        $this->updateKinopoisk();
    }

    /* 
    * Parsing comma separated string of countries and attaching them to movie 
    */
    public function attachCountries($countries){

        foreach($countries as $item) {
            $name = mb_strtolower(trim($item), 'UTF-8');

            $country = Country::where('name', $name)->first();

            if ( empty($country) ) {
                $country = new Country();
                $country->fill(['name' => $name])->save();
            }

            $movie->countries()->attach($country->id);
        }
    }

    /*
     * Update Kinopoisk information
     */
     public function updateKinopoisk(){}

    /*
     * Directors
     */
     public function attachDirectors($directors){ ... }

     /*
      * Actores
      */
      public function attachActors($actors){ ... }

      /*
       * Genders
       */
       public function attachActors($actors){ ... }
}

Poster, you may considere using a service provider (I will show this example because I do not know your Poster model looks like):

public class PosterManager{

    public static function upload($file, $movie){
        $poster = \Image::make($file);
        $poster->fit(250, 360, function ($constraint) {
            $constraint->upsize();
        });

         $path = config('app.images') . $movie->id.'/';

         if(! \File::exists($path)) {
            \File::makeDirectory($path);
        }

        $filename = time() . '.' . $file->getClientOriginalExtension();
        $poster->save($path . $filename);

        return $poster;
    }
}

Config file Try using config files to store relevant application constanst/data, for example, to store movie images path:

'images' => storage_path() . '/images/movies/';

Now, you are able to call $path = config('app.images'); globally. If you need to change the path only setting the config file is necessary.

Controllers as injected class. Finally, the controller is used as a class where you only need to inject code:

public function store(CreateMovieRequest $request) {
    $movie = Movies::create($request->except('poster'));

    /* Uploading poster */
    if ($request->hasFile('poster')) {
        $file = $request->file('poster');
        $poster = \PosterManager::upload($file, $movie);
        $movie->poster = $poster->filename;
    }

    if (!empty($request->input('genres'))) {
        $genres = explode(',', $request->input('genres'));

        $movie->attachGenders($genders);
    }

    // movie->attachDirectors();
    // movie->attachCountries();

    // Apply all changes
    $movie->save();

    return redirect('/admin/movies');
}