7

I'm trying to make a simple "friends" system, by storing each friend id in a single column as a string separated with a comma.

I want to get the users by id and store them in an array:

public function getFriends() {
    $friendids = explode(',', $this->friends);
    $friends = [];

    foreach ($friendids as $id) {
        $friends[] = User::findOrFail($id);
    }

    return $friends;
}

So I can do this in my view:

@foreach ($user->getFriends() as $friend)
    <p>Friend ID: {{ $friend->id }}</p>
    <p>Friend Username: {{ $friend->username }}</p>
@endforeach

But I think I can't use findOrFail in the user model or what am I doing wrong?

3 Answers3

14

To use the findOrFail($id) method, you should invoke the object type you want to get. Example:

foreach ($friendids as $id) {
        $friends[] = User::findOrFail($id);
    }
Fudge
  • 49
  • 4
Pastor Durán A.
  • 364
  • 1
  • 12
  • Ah yes, that was a simple mistake. I actually did have the object type before, but I must've removed it by accident when searching for a mistake. The problem was actually, that I had specified user id's in my database, that do not exist and I couldn't read the error correctly, smh. Fixed in main post, if anyone would ever want to use the same method. –  Sep 07 '15 at 18:19
  • 1
    This is incredibly inefficient. For each friend, you're running a query. That doesn't scale very well when each user has hundreds of friends. – Amo Sep 08 '15 at 08:08
4

Although there's an existing answer, I wanted to provide an alternative solution that I personally feel is a better long-term solution.

Firstly, saving friends as an comma separated list isn't going to scale very well. It also massively limits the ability for you to do things such as 'friends of friends', and other more complex queries.

Really, you should have two tables, users and friends.

User model

public function acceptedFriends()
{
    return $this->hasMany('App\Friend')->where('accepted', true);
}

public function pendingFriends()
{
    return $this->hasMany('App\Friend')->where('accepted', false);
}

Now, when a friend request is rejected, you can simply delete the record in the Friends table.

Loading a user with their friends:

$user = User::find($id)->with('acceptedFriends')->firstOrFail();

Loading a user's friend requests:

$users = Friends::where('accepted', false)->where('target_user', $userId)->get();

You might want to also check out https://stackoverflow.com/a/25057320/972370. It's written by someone who is very well known in the Laravel community for advanced Eloquent usage.

Community
  • 1
  • 1
Amo
  • 2,884
  • 5
  • 24
  • 46
  • I really do appreciate the reply, however the current project I'm working on, does not need that complex queries. I'll be considering this however, when I'm going to work on something like that in the future. Thanks! –  Sep 07 '15 at 18:46
  • IMO, this is less complex and the laravel way of doing it. – Samuel De Backer Sep 07 '15 at 19:25
  • Does `$user::find($id)` fire a query to the DB behind the scenes? Or is that loaded from the cache/session somewhere? – nclsvh Mar 26 '18 at 14:03
1

I made 2 simple mistakes:

1) I removed User object from findOrFail()when looking for the mistake I made

2) I had specified user id's (friends) that did not exist in the databse, causing an error

I also improved the code a bit, by adding an optional parameter to get only x amount of users:

// App/User.php
public function getFriends($friends_to_get = null) {
    $friendids = explode(',', $this->friends);
    $friends = [];

    $i = 0;
    foreach ($friendids as $id) {
        if (is_numeric($id)) {
            $friends[] = User::findOrFail($id);
        }

        if (isset($friends_to_get) && $i < $friends_to_get) {
            ++$i;
        }

        if (isset($friends_to_get) && $i == $friends_to_get) {
            break;
        }
    }

    return $friends;
}

Now you can do something like this in your view:

@foreach ($user->getFriends() as $friend)
    <p>Friend ID: {{ $friend->id }}</p>
    <p>Friend Username: {{ $friend->username }}
@endforeach

Or if you want to get, for example, 6 friends only, you can do: $user->getFriends(6)

In my case, $user is the user of who's profile I'm currently viewing. If you want, you could also get your friends only, by doing Auth::user()->getFriends()

/Edit: Added is_numeric check in the foreach loop. This way you don't have to worry about first and last element in the column having an extra comma. Since explode adds an extra value to the array, if there's a comma in the end of the friends column. Now you can just add or remove x, from/to the column every time and not worry about fetching an object, that does not exist.