在Laravel有更好的方法吗?

在Laravel有更好的方法吗?

问题描述:

I am pretty new to Laravel and I have been coding a CMS for a roleplay game, and I have ran in to some code which I see as messy / bad practice, I was just wondering is this really the easiest and best way to do it? What I am mainly looking for is the best practice for doing this in the best possible way without changing too much of my code.

So what I need to do is I need to pass the users business and the users business position to the business overview view/page. I have modals for each of the tables for Business and BusinessPositions which I will post below.

Businesses:

<?php
namespace App\Database\Website\Roleplay\Business;

use Eloquent;

class Businesses extends Eloquent
{
    protected $primaryKey   = 'id';
    protected $table        = 'srp_businesses';
    public $timestamps      = false;
    protected $fillable     = [];

    public function positions()
    {
      return $this->hasMany('App\Database\Roleplay\Business\BusinessPositions', 'business_id');
    }

    public function founder()
    {
      return $this->belongsTo('App\Database\Website\User\Player');
    }
}

BusinessPositions:

<?php
namespace App\Database\Website\Roleplay\Business;

use Eloquent;

class BusinessPositions extends Eloquent
{
    protected $primaryKey   = 'id';
    protected $table        = 'srp_business_positions';
    public $timestamps      = false;
    protected $fillable     = [];
}

And here is the controller function for sending my view

$businesses = Cache::remember('overview.businesses', 1, function() {
    return Businesses::get();
});

$businessPositions = Cache::remember('overview.business.positions', 1, function() {
    return BusinessPositions::get();
});

$acceptedCount = $businesses->where('business_owner_id', '=', Auth::user()->id)
    ->where('business_status', '=', 'accepted')
    ->count();
$pendingCount = $businesses->where('business_owner_id', '=', Auth::user()->id)
    ->where('business_status', '=', 'pending')
    ->count();
$myBusiness = Businesses::find(Auth::user()
    ->roleplay->business_id);
$myBusinessPosition = $businessPositions->where('business_id', '=', $myBusiness->business_id)
    ->where('position_id', '=', Auth::user()->roleplay->business_position)
    ->first();

return view('frontend.business.overview', compact('acceptedCount', 'pendingCount', 'myBusiness', 'myBusinessPosition'));

The part I see as messy is the way I get the business position, and maybe there is a way to get the business better?

How I get the business position (breakdown of code)

$myBusinessPosition = $businessPositions->where('business_id', '=', $myBusiness->business_id)
    ->where('position_id', '=', Auth::user()
        ->roleplay->business_position)
    ->first();

I am using Laravel 5.3, Thanks.

You can use scopes, click here to go and read the docs on this.

Doesn't this look much cleaner?

$user->businesses()->ofStatus('accepted')->count();
$user->businesses()->ofStatus('pending')->count();

Also, you should fix up your calls to the auth, instead of making raw calls like this.

->where('business_owner_id', '=', Auth::user()->id)

Create a variable, and safe it to use it further down the scope of your code.

$user = Auth::user()

Then, just use the user, not the auth code.

->where('business_owner_id', '=', $user->id)

Just a few tips:

Scopes and relationships

You could probably introduce a direct relationship in the User model to businesses.

By using query scopes, your code would read easier:

$user->businesses()->accepted()->count();
$user->businesses()->pending()->count();

Or if you use just 1 dynamic scope:

$user->businesses()->ofStatus('accepted')->count();
$user->businesses()->ofStatus('pending')->count();

Using collections

In that case, you could count all businesses with just a single SQL query (filtering here happens in memory, returns a collection where you got counts keyed by the status):

$countByStatus = $user->businesses
    ->pluck('status')
    ->groupBy(function($item) { return $item; }) 
    ->flatMap(function($item, $key) { return [$key => $item->count()]; });