0
\$\begingroup\$

I have a user table like this

id | firstname | surname  | title 
1  | Bob       | Batman   | Dr.
2  | Alice     | Grimley  | Prof.
3  | Clara     | Caprio   |

In the user model class I have a function that computes the name

class User extends Authenticatable
{
    //...

    public function computeName()
    {
      return  trim($this->title . ' ' . $this->firstname . ' ' . $this->surname);
    }

    //...
}

In addition I have a 1:n relation with a table sortUser2Pres where n can only be 0 or 1. The relation maps if a user is a president or vice-president in a country.

user_id | country | type 
1       | DE      | vice-president
2       | DE      | president

Now I wish to compute a name list of all users who are president. I can achieve it as follows:

$presidents = DB::table('user')
               ->select('firstname','surname','title')
               ->join('sortUser2Pres s', 's.user_id','=','user.id')
               ->where('s.type','president')
               ->get();

$names =  [];
foreach ($presidents as $user) {
    $names[] = trim($->title . ' ' . $->firstname . ' ' . $user->surname);
}

However, I think its bad that I cannot make use of the functions from the user model like computeName and that I have to copy & paste this function. Isn't there a cleaner way of achieving such a name list in Laravel and making use of the existing functions in the user model?

\$\endgroup\$
  • \$\begingroup\$ Dr. Bob Batman here - I don't recall giving you permission to use my name. \$\endgroup\$ – I wrestled a bear once. Oct 10 '17 at 20:27
  • \$\begingroup\$ @Iwrestledabearonce. sorry I did't use your name, this is a real backup from my porn site user table. \$\endgroup\$ – Adam Oct 10 '17 at 20:40
2
\$\begingroup\$

why do you have a 1:n relation? you can just add a 1:1 or column to your user table which has a type column for example and it holds president or vice-president values and then you can have a function in your model to check if it's a president or vice-president.

You can add a function to your repository to get all user which they are president with a simple query.

\$\endgroup\$
  • 1
    \$\begingroup\$ Your right a 1:n relation is unnecessary one can use the hasOne relation. And my question was basically how to write the simple query for that, but I found out now. \$\endgroup\$ – Adam Oct 22 '17 at 9:15
0
\$\begingroup\$

Seems like your main problem is that you cannot reuse computeName() and that's because you've decided to couple it to the user model object, the nail in the coffin for that coupling was using $this there's a few ways to uncouple that at run time but you'd be better off without the coupling in the first place:

Since it seems like you need computeName() outside of the context of the model object I think it's time to decouple your method into its own function and then reuse it wherever you need it :

function compute_name(string $title, string $firstname, string $surname) : string
{
  return trim($title . ' ' . $firstname . ' ' . $surname);
}

I'd also consider renaming the function, 'compute' seems redundant, as does the trim() function unless you're absolutely sure you have white space at the beginning of your titles or end of your surnames, in which case I would ask why that wasn't stripped before it reached the database

\$\endgroup\$
  • \$\begingroup\$ Thanks for your suggestion, but I did not want to uncouple the method, I just wanted to know how to use the query, but I found out by now. I think your right, computeName is a bad function name. Probably better to use something like fullName(). The trim() function is necessary though. The input in my database is stripped, but not every user has a title so I have to trim ($title . ' ' . $name \$\endgroup\$ – Adam Oct 22 '17 at 9:31
  • \$\begingroup\$ You could add fullName() to your object as long as you stop using $this in your method, it can be used it both call points by supplying the arguments $user->fullName($title, $firstname, $surname); \$\endgroup\$ – arcanine Oct 22 '17 at 12:41
0
\$\begingroup\$

Here is how I solved it:

I used a hasOne relation in the user class because it can be used if you need a hasOneOrZero relation ( see https://stackoverflow.com/questions/46866046/does-laravel-have-a-one-to-zero-or-one-eloquent-relation)

public function president()
{
  return $this->hasOne('App\Profile\President','idMember');
}

Now that I didn't have a pivot table anymore, I stored the type column in the president table and thus I could change the query to

$presidents = User::select('firstname','surname','title')
                    ->whereHas('president', function($q){
                       $q->where('type','president')
                     })
                     ->get();


$names =  [];
foreach ($presidents as $user) {
    $names[] = $user->computeName();
}

By the way, if you wanted to keep the relation as a 1:n relation than the query should be called like this:

$presidents = User::select('firstname','surname','title')
                    ->whereHas('president', function($q){
                       $q->where('sortUser2Pres.type','president')
                     })
                     ->get();

where sortUser2Pres is the pivot table.

\$\endgroup\$

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.