CODEWITHSUNDEEP
phplaravellaravel-5
Paradigm
Paradigm

Reputation: 371

Laravel best practice - Query

I need to pass some data down to my view. I have two models a user and a tips model.

The User model has a method that returns the user hasMany(Tip::Class) and the Tip model has a method that returns that the tip belongsTo(User::class).

I'm doing a profile page for a user and using route model binding to return a user model when accessing the profile.

public function tipsterProfileShow(User $tipster)
{
  if (!$tipster->isTipster())
  {
    return redirect()->route('home');
  }

  return view('profile.index')->with([
    'tipster' => $tipster,
    'tips' => $tipster->tips(),
  ]);
}

I want to display some data such as the amount of tips that are correct which are indicated by the status column in the tips table.

At the moment in the blade view I'm using 

{{$tips->where('status','Won')->count()}}

I feel this isn't best practice but I may be wrong.

Would it be better doing something like the below?

public function tipsterProfileShow(User $tipster)
{
  if (!$tipster->isTipster())
  {
    return redirect()->route('home');
  }
  return view('profile.index')->with([
    'tipster' => $tipster,
    'tips' => $tipster->tips(),
    'wins' => $tipster->tips()->where('status', 'Won')->count()
  ]);
}

That way I would be keeping the queries out of the view. I'm really new to laravel and 'best practice' so trying to get some advice.

Upvotes: 1

Views: 834

Answers (3)

henrik
henrik

Reputation: 1618

Yes, you are correct. Given you are trying to follow the MVC software development pattern, you should avoid putting business logic in your view.

This line of code:

{{$tips->where('status','Won')->count()}}

is actually doing 2 things:

  1. Query the model for all objects with a certain criteria
  2. Count the result

By following the MVC principles it should be your controller that sends the commands to the model, not the view.

Good luck!

Upvotes: 0

Ghost Worker
Ghost Worker

Reputation: 690

I will advice you creating a POPO (Plain Old PHP Object) that will contain a full description of a user more synonymous to a User profile.

A model in laravel represents a row in the table and it will lazy-load any relationship attached to it.

So if you have a POPO, you will be able to define everything that is related to the user and pass it on to the view without having to query in the view.

Take below as an example:

Class UserPorfile{
  private $id;
  private $username;
  private $tips;

  public function setId($id){
     $this->id = $id;
  }

  public function getId(){
     return $this->id;
  }

  public function setUsername($username){
      $this->username = $username;
  }

  public function getUsername(){
     return $this->username;
  }

  public function setTips(array $tips){
     $this->tips = $tips;
  }

  public function getTips(){
     return $this->tips;
  }
}

passing an object of this class to your view seems much more better

Upvotes: 0

Joel Hinz
Joel Hinz

Reputation: 25384

You're asking about a best practice, which is generally frowned upon, but this really is something essential that every beginner should learn, so I still think it merits an answer.

In brief: YES! What you're doing is a great first step towards keeping your code separated by logic. Your views should be responsible for displaying data, and your controllers for handling data over to the views. Whether your controllers should actually be responsible for calculating the data is another topic, and one which is constantly debated.

That said, you could get this down to just a single line in the controller if you apply a little bit of other logic:

public function tipsterProfileShow(User $tipster)
{
  return view('profile.index', compact('tipster'));
}

The first step is to add a method to your User model, something like this:

public function winCount()
{
    return $this->tips()->where('status', 'Won')->count();
}

Now you can access $tipster->winCount() from your view. You can also access $tipster->tips() straight away in your view - most would agree that's perfectly fine.

The second step is to extract the redirect call for non-tipsters into a middleware, which you can read about here: https://laravel.com/docs/5.3/middleware

There are further steps you might take from there, but that's a good starting point. Good luck! :)

Upvotes: 3

Related Questions