3
\$\begingroup\$

I have an Eloquent model that has multiple types of joins that is used to filter out different request to grab the data of movies e.g.: Actors, Directors, Writers, etc...

The joins are necessary because they use unconventional pivot tables. All the queries and eloquent here works and gives the desired returned data - however I'm breaking DRY (Don't Repeat Yourself) principle for each join filters I am creating within a controller.

Here are the code segments that I would like to tweak:

This is a call for all movies by actors:

public function getByActor($where, $limit, $offset)
{
    $movieData = array();

    $restrictedIdList = $this->getRestrictedIds();
    if (empty($restrictedIdList)) {
        $restrictedIdList = array(0);
    }

    $movieData = Movie::
        FilterByActorWithInnerJoin(urldecode($where['actorName']))
        ->selectRaw('movie.*')
        ->where('movie.status','=','active')
        ->whereNotIn('movie.id', $restrictedIdList)
        ->take($limit)
        ->skip($offset)
        ->get()
        ->toArray();
    return $movieData;
}

This is for Writers:

public function getByWriter($where, $limit, $offset)
{
    $movieData = array();

    $restrictedIdList = $this->getRestrictedIds();
    if (empty($restrictedIdList)) {
        $restrictedIdList = array(0);
    }

    $movieData = Movie::
        FilterByWriterWithInnerJoin(urldecode($where['writerName']))
        ->selectRaw('movie.*')
        ->where('movie.status','=','active')
        ->whereNotIn('movie.id', $restrictedIdList)
        ->take($limit)
        ->skip($offset)
        ->get()
        ->toArray();
    return $movieData;
}

As you can see the only difference is the FilterBy<filter type join>WithInnerJoin that I have created within my model.

Here are the joins of the model:

public function scopeFilterByWriterWithInnerJoin($query,$writerName)
{
    $query->join('movie_writers as mw', 'mw.movie_id', '=', 'movie.id')
        ->join('writers as w',function($join) use ($writerName){
            $join->on('w.id', '=', 'mw.writer_id')
                ->where('w.name','like','%'.$writerName.'%');
        });
}

public function scopeFilterByActorWithInnerJoin($query,$actorName)
{
    $query->join('movie_actors as ma', 'ma.movie_id', '=', 'movie.id')
        ->join('actors as a',function($join) use ($actorName){
            $join->on('a.id', '=', 'ma.actor_id')
                ->where('a.name','like','%'.$actorName.'%');
        });
}

I will have to do the same for producers, directors, etc.

Is there a more efficient way to do this?

\$\endgroup\$
2
\$\begingroup\$

I don't see a way to do it with eager loads. I wonder if you could reduce it to getBy($k, $v) using distinct though?

class Movie extends Eloquent
{

    function writers()
    {
        return $this->belongsToMany('Writer');
    }

    function actors()
    {
        return $this->belongsToMany('Actor');
    }

    /**
     * Get a list of Movies with the matching the key-value.
     * 
     * @param string $k Key (movie attribute) name
     * @param string $v Value of the movie attribute
     *
     * @return \Illuminate\Database\Query\Builder
     */
    function getBy($k, $v) {
        return Movie::join('movie_writers as mw', 'mw.movie_id', '=', 'movie.id')
            ->join('writers as w', 'w.id', '=', 'mw.writer_id')
            ->join('movie_actors as ma', 'ma.movie_id', '=', 'movie.id')
            ->join('actors as a', 'a.id', '=', 'mw.actor_id')
            ->where('movie.status','=','active')
            ->select('movie.id', 'movie.name')
            ->where($k, 'like', "%$v%")
            ->distinct();
    }
}

With this in place, getBy will return a Query Builder object that you can filter by restricted and limit.

$movie = new Movie();
$movies = $movie->getBy('writers.name', 'Matthew Weiner')
                ->whereNotIn('movie.id', $this->restricted())
                ->take($limit)
                ->skip($offset)
                ->get();

Then you can take the a step further by pulling this method out into a MovieRepository that implements MovieRepositoryInterface. Check out this Laracasts episode for more on that. It could have a MovieRepository->restrictedBy(Builder $builder, $restrictedList) that also returns a Builder object.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Forgive me, I've never written a word of PHP, and all of this under the assumption of possible.

Making NonRestricted it's own scope would be a first good start.

public function scopeNonRestricted( $query )
{
  $restrictedIdList = $this->getRestrictedIds();
  if (empty($restrictedIdList)) {
      $restrictedIdList = array(0);
  }

  query->where('movie.status','=','active')
       ->whereNotIn('movie.id', $restrictedIdList)
}

The join functions are so similar, I also see that PHP queries can be strings:

public function scopeInnerJoin($query, $table, $value)
{
  // For example, called with $table equal 'actor'
  $table_name = $table + 's'
  $join_table = 'movie_' + $table_name + ' as jt'
  $join_on    = $table_name + ' as jo'
  $table_id   = 'jt.' + $table + '_id'

  $query->join( $join_table, 'jt.movie_id', '=', 'movie.id')
    ->join( $join_on ,function($join) use ($value){
      $join->on('jo.id', '=', $table_id )
      ->where('jo.name','like','%'.$value.'%');
    });
}

Coupled with a NonRestricted scope, you might have something like this:

public function getBy($table, $where, $limit, $offset)
{
  // I took out the $movieData declaration...
  return Movie::
    NonRestricted::  // possible?
    InnerJoin( $table, $where )
    ->take($limit)
    ->skip($offset)
    ->get()
    ->toArray();
}

public function getByActor($where, $limit, $offset)
{
  return getBy( 'actor', urldecode($where['actorName']) )
}
public function getByWriter($where, $limit, $offset)
{
  return getBy( 'writer', urldecode($where['writerName']) )
}

Again, forgive me for not knowing any PHP. You may even consider making 'active' it's own scope as well, what good is it for an admin not to be able to find inactive and restricted movies? I would probably make the pagination it's own scope as well.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ If you havent written PHP code before, then what are you basing your answer on? \$\endgroup\$ – azngunit81 Sep 22 '14 at 5:37
  • \$\begingroup\$ For the InnerJoin, just some string manipulation. For the scoping, Ruby on Rails. Coming from RoR, I would make a scope for active, restricted, pagination, a straight up inner join, parameters for the join, and where clauses. \$\endgroup\$ – Carl Sep 23 '14 at 20:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.