Laravel UserController Design
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
4
down vote
favorite
<?php
namespace AppHttpControllers;
use IlluminateHttpRequest;
use AppUser;
use Auth;
class ProfileController extends Controller
public function __construct()
$this->middleware('auth');
public function index($id)
$user = User::find($id);
$photos = $user->getPhotos();
$interests = $user->getInterests();
$gifts = $user->getGifts();
$visits = $user->totalVisits();
$likes = $user->totalLikes();
if ($user->id == Auth::user()->id)
$page_name = 'My Profile';
return view('profile.my_profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
else
$page_name = $user->name;
return view('profile.profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
I have the following controller, which manages the "Profile" page in my web app. I feel like in my current code, there is a separation in a way that photos, interests, gifts, visits and likes look like they do not belong to the User.
How would I go to refactor all these different functions into one? Should I change the User Model's __construct to execute all of the functions? Ideally, I would like to pass only two variables to my view ("user" and "page_name" variables)
Here is my model, for reference:
<?php
namespace App;
use IlluminateFoundationAuthUser as Authenticatable;
use IlluminateNotificationsNotifiable;
use IlluminateSupportFacadesDB;
class User extends Authenticatable
use Notifiable;
protected $table = 'users';
public $timestamps = false;
/**
* The attributes that are mass assignable.
*
* @var array
*/
protected $fillable = [
'name', 'email', 'password',
];
/**
* The attributes that should be hidden for arrays.
*
* @var array
*/
protected $hidden = [
'password', 'remember_token',
];
public function getPhotos()
return DB::table('photos')
->where('user_id', $this->id)
->get();
public function getInterests()
return DB::table('interests')
->where('user_id', $this->id)
->get();
public function getGifts()
return DB::table('gifts')
->join('sent_gifts', function ($join)
$join->on('gifts.id', '=', 'sent_gifts.gift_id')
->where('user_2', $this->id);
)
->join('users', function ($join)
$join->on('users.id', '=', 'sent_gifts.user_1');
)
->get();
public function getVisits()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'visit');
)
->get();
public function getLikes()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'like');
)
->get();
public function getFilter()
return DB::table('filters')
->where('user_id', $this->id)
->limit(1)
->get();
public function totalVisits()
return DB::table('user_reactions')
->where('reaction', 'visit')
->where('user_2', $this->id)
->get()
->count();
public function totalLikes()
return DB::table('user_reactions')
->where('reaction', 'like')
->where('user_2', $this->id)
->get()
->count();
php mvc laravel
add a comment |Â
up vote
4
down vote
favorite
<?php
namespace AppHttpControllers;
use IlluminateHttpRequest;
use AppUser;
use Auth;
class ProfileController extends Controller
public function __construct()
$this->middleware('auth');
public function index($id)
$user = User::find($id);
$photos = $user->getPhotos();
$interests = $user->getInterests();
$gifts = $user->getGifts();
$visits = $user->totalVisits();
$likes = $user->totalLikes();
if ($user->id == Auth::user()->id)
$page_name = 'My Profile';
return view('profile.my_profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
else
$page_name = $user->name;
return view('profile.profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
I have the following controller, which manages the "Profile" page in my web app. I feel like in my current code, there is a separation in a way that photos, interests, gifts, visits and likes look like they do not belong to the User.
How would I go to refactor all these different functions into one? Should I change the User Model's __construct to execute all of the functions? Ideally, I would like to pass only two variables to my view ("user" and "page_name" variables)
Here is my model, for reference:
<?php
namespace App;
use IlluminateFoundationAuthUser as Authenticatable;
use IlluminateNotificationsNotifiable;
use IlluminateSupportFacadesDB;
class User extends Authenticatable
use Notifiable;
protected $table = 'users';
public $timestamps = false;
/**
* The attributes that are mass assignable.
*
* @var array
*/
protected $fillable = [
'name', 'email', 'password',
];
/**
* The attributes that should be hidden for arrays.
*
* @var array
*/
protected $hidden = [
'password', 'remember_token',
];
public function getPhotos()
return DB::table('photos')
->where('user_id', $this->id)
->get();
public function getInterests()
return DB::table('interests')
->where('user_id', $this->id)
->get();
public function getGifts()
return DB::table('gifts')
->join('sent_gifts', function ($join)
$join->on('gifts.id', '=', 'sent_gifts.gift_id')
->where('user_2', $this->id);
)
->join('users', function ($join)
$join->on('users.id', '=', 'sent_gifts.user_1');
)
->get();
public function getVisits()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'visit');
)
->get();
public function getLikes()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'like');
)
->get();
public function getFilter()
return DB::table('filters')
->where('user_id', $this->id)
->limit(1)
->get();
public function totalVisits()
return DB::table('user_reactions')
->where('reaction', 'visit')
->where('user_2', $this->id)
->get()
->count();
public function totalLikes()
return DB::table('user_reactions')
->where('reaction', 'like')
->where('user_2', $this->id)
->get()
->count();
php mvc laravel
2
It looks like you're not using Laravel relationships which reduce a lot of your code in the User model. See here laravel.com/docs/5.6/eloquent-relationships
â Josh Griggs
May 14 at 12:28
What version of Laravel are you using?
â Ross Wilson
May 30 at 7:39
@RossWilson Laravel 5.4
â garjoka madafa
May 30 at 17:44
add a comment |Â
up vote
4
down vote
favorite
up vote
4
down vote
favorite
<?php
namespace AppHttpControllers;
use IlluminateHttpRequest;
use AppUser;
use Auth;
class ProfileController extends Controller
public function __construct()
$this->middleware('auth');
public function index($id)
$user = User::find($id);
$photos = $user->getPhotos();
$interests = $user->getInterests();
$gifts = $user->getGifts();
$visits = $user->totalVisits();
$likes = $user->totalLikes();
if ($user->id == Auth::user()->id)
$page_name = 'My Profile';
return view('profile.my_profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
else
$page_name = $user->name;
return view('profile.profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
I have the following controller, which manages the "Profile" page in my web app. I feel like in my current code, there is a separation in a way that photos, interests, gifts, visits and likes look like they do not belong to the User.
How would I go to refactor all these different functions into one? Should I change the User Model's __construct to execute all of the functions? Ideally, I would like to pass only two variables to my view ("user" and "page_name" variables)
Here is my model, for reference:
<?php
namespace App;
use IlluminateFoundationAuthUser as Authenticatable;
use IlluminateNotificationsNotifiable;
use IlluminateSupportFacadesDB;
class User extends Authenticatable
use Notifiable;
protected $table = 'users';
public $timestamps = false;
/**
* The attributes that are mass assignable.
*
* @var array
*/
protected $fillable = [
'name', 'email', 'password',
];
/**
* The attributes that should be hidden for arrays.
*
* @var array
*/
protected $hidden = [
'password', 'remember_token',
];
public function getPhotos()
return DB::table('photos')
->where('user_id', $this->id)
->get();
public function getInterests()
return DB::table('interests')
->where('user_id', $this->id)
->get();
public function getGifts()
return DB::table('gifts')
->join('sent_gifts', function ($join)
$join->on('gifts.id', '=', 'sent_gifts.gift_id')
->where('user_2', $this->id);
)
->join('users', function ($join)
$join->on('users.id', '=', 'sent_gifts.user_1');
)
->get();
public function getVisits()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'visit');
)
->get();
public function getLikes()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'like');
)
->get();
public function getFilter()
return DB::table('filters')
->where('user_id', $this->id)
->limit(1)
->get();
public function totalVisits()
return DB::table('user_reactions')
->where('reaction', 'visit')
->where('user_2', $this->id)
->get()
->count();
public function totalLikes()
return DB::table('user_reactions')
->where('reaction', 'like')
->where('user_2', $this->id)
->get()
->count();
php mvc laravel
<?php
namespace AppHttpControllers;
use IlluminateHttpRequest;
use AppUser;
use Auth;
class ProfileController extends Controller
public function __construct()
$this->middleware('auth');
public function index($id)
$user = User::find($id);
$photos = $user->getPhotos();
$interests = $user->getInterests();
$gifts = $user->getGifts();
$visits = $user->totalVisits();
$likes = $user->totalLikes();
if ($user->id == Auth::user()->id)
$page_name = 'My Profile';
return view('profile.my_profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
else
$page_name = $user->name;
return view('profile.profile', compact('page_name', 'user', 'photos', 'interests', 'likes', 'visits', 'gifts'));
I have the following controller, which manages the "Profile" page in my web app. I feel like in my current code, there is a separation in a way that photos, interests, gifts, visits and likes look like they do not belong to the User.
How would I go to refactor all these different functions into one? Should I change the User Model's __construct to execute all of the functions? Ideally, I would like to pass only two variables to my view ("user" and "page_name" variables)
Here is my model, for reference:
<?php
namespace App;
use IlluminateFoundationAuthUser as Authenticatable;
use IlluminateNotificationsNotifiable;
use IlluminateSupportFacadesDB;
class User extends Authenticatable
use Notifiable;
protected $table = 'users';
public $timestamps = false;
/**
* The attributes that are mass assignable.
*
* @var array
*/
protected $fillable = [
'name', 'email', 'password',
];
/**
* The attributes that should be hidden for arrays.
*
* @var array
*/
protected $hidden = [
'password', 'remember_token',
];
public function getPhotos()
return DB::table('photos')
->where('user_id', $this->id)
->get();
public function getInterests()
return DB::table('interests')
->where('user_id', $this->id)
->get();
public function getGifts()
return DB::table('gifts')
->join('sent_gifts', function ($join)
$join->on('gifts.id', '=', 'sent_gifts.gift_id')
->where('user_2', $this->id);
)
->join('users', function ($join)
$join->on('users.id', '=', 'sent_gifts.user_1');
)
->get();
public function getVisits()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'visit');
)
->get();
public function getLikes()
return DB::table('user_reactions')
->join('users', function ($join)
$join->on('users.id', '=', 'user_reactions.user_1')
->where('user_2', $this->id)
->where('reaction', 'like');
)
->get();
public function getFilter()
return DB::table('filters')
->where('user_id', $this->id)
->limit(1)
->get();
public function totalVisits()
return DB::table('user_reactions')
->where('reaction', 'visit')
->where('user_2', $this->id)
->get()
->count();
public function totalLikes()
return DB::table('user_reactions')
->where('reaction', 'like')
->where('user_2', $this->id)
->get()
->count();
php mvc laravel
asked May 14 at 5:45
garjoka madafa
233
233
2
It looks like you're not using Laravel relationships which reduce a lot of your code in the User model. See here laravel.com/docs/5.6/eloquent-relationships
â Josh Griggs
May 14 at 12:28
What version of Laravel are you using?
â Ross Wilson
May 30 at 7:39
@RossWilson Laravel 5.4
â garjoka madafa
May 30 at 17:44
add a comment |Â
2
It looks like you're not using Laravel relationships which reduce a lot of your code in the User model. See here laravel.com/docs/5.6/eloquent-relationships
â Josh Griggs
May 14 at 12:28
What version of Laravel are you using?
â Ross Wilson
May 30 at 7:39
@RossWilson Laravel 5.4
â garjoka madafa
May 30 at 17:44
2
2
It looks like you're not using Laravel relationships which reduce a lot of your code in the User model. See here laravel.com/docs/5.6/eloquent-relationships
â Josh Griggs
May 14 at 12:28
It looks like you're not using Laravel relationships which reduce a lot of your code in the User model. See here laravel.com/docs/5.6/eloquent-relationships
â Josh Griggs
May 14 at 12:28
What version of Laravel are you using?
â Ross Wilson
May 30 at 7:39
What version of Laravel are you using?
â Ross Wilson
May 30 at 7:39
@RossWilson Laravel 5.4
â garjoka madafa
May 30 at 17:44
@RossWilson Laravel 5.4
â garjoka madafa
May 30 at 17:44
add a comment |Â
1 Answer
1
active
oldest
votes
up vote
4
down vote
accepted
First of all do not use Laravel 5.4 even if you've written some* code already, use latest 5.6, and try to always update.
*: You really need to rewrite the application if you have code like you presented everywhere, so just take your time and use L5.6. Idea is already there you just need to improve it - pure coding, no thinking.
Model(s)
Standards
You are sticking to the standard (I guess because User model is the stock one) and using class name User
for a model means that table is plural which renders $table
property useless and you save one line.
Each table shall have its own Model.
Note that we did not tell Eloquent which table to use for our Flight model. By convention, the "snake case", plural name of the class will be used as the table name unless another name is explicitly specified.
Relationships
Main reason you have read about Laravel when you were googling around which framework to use is because its ORM Eloquent*. Eloquent is a abstraction so you do not even need to know how each relationship work under the hood, it does all the magic for you. Firstly you'll need to understand what relationships are so google around, and when you are done use superb documentation of Laravel** and get all the info from there.
*,**: In early days of Laravel it was the selling point for many developers.
Defining the relationships is hard part of RDBMS design, so take your time, do it right, use pen and paper and then code it - easy part if design is right.
Note: For
user_reactions
use polymorphic relations.
Invest time to learning relationships it will pay off soon :)
Note: I am not writing about how I would define relations you have in code because of lack of context and mainly because of helping too much and I consider it as giving a fish instead of teaching how to fish.
Controller
Routing / Middleware / Route-Model-Binding
Forget about defining middlewares in controller itself, use routes file for that, so delete the __construct()
and just add it back to the routes file. You may ask why, well because of DRY, you will end up with 5-x controllers and each of them will contain $this->middleware('auth');
in constructor, repeating...*
*: Unless you make a "master" class like AuthController and extend from there - IMO that just adds more complications than good if you want to define middleware only.
Use groups in route file, and define middleware on the groups part, see below for route example.
Route Model Binding another super useful feature of Laravel. Example vs bunch of words:
Route:
Route::middleware(['auth'])->group(function ()
Route::get('profile/user', 'ProfileController@index'); <-- Route Model Binding
Route::get('profile/edit', 'ProfileController@edit');
);
Signature of ProfileController@index
method should be index(AppUser $user)
, you see behind the scenes Laravel does query to the database and fetches User
instance for you.
After defining all the relationships on User model you will most likely end up with using a oneliner to get the $user
:
$user = User::with(['photos', 'interests', 'gifts', 'visits', 'likes'])->whereId($id)->get(); // if you stick with not using Route Model Binding
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']); // if you use Route Model Binding
// in view you will access interests for example $user->interests
Now what about edge case if user is looking at their own profile, as I understand the only difference is to change variable $page_name
, do it right in the view (I would do it that way):
@if($user->id == Auth::user()->id)
My Profile
@else
$user->name
@endif
So proper controller after suggestions would look like:
<?php
namespace AppHttpControllers;
use AppUser;
use Auth;
class ProfileController extends Controller
public function index(User $user)
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']);
return view('profile.my_profile', compact('user'));
Packages
Do not reinvent the wheel, for example for handling attachments / images use solution that is tested https://github.com/spatie/laravel-medialibrary and used by many!
During development debugbar is a must.
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
4
down vote
accepted
First of all do not use Laravel 5.4 even if you've written some* code already, use latest 5.6, and try to always update.
*: You really need to rewrite the application if you have code like you presented everywhere, so just take your time and use L5.6. Idea is already there you just need to improve it - pure coding, no thinking.
Model(s)
Standards
You are sticking to the standard (I guess because User model is the stock one) and using class name User
for a model means that table is plural which renders $table
property useless and you save one line.
Each table shall have its own Model.
Note that we did not tell Eloquent which table to use for our Flight model. By convention, the "snake case", plural name of the class will be used as the table name unless another name is explicitly specified.
Relationships
Main reason you have read about Laravel when you were googling around which framework to use is because its ORM Eloquent*. Eloquent is a abstraction so you do not even need to know how each relationship work under the hood, it does all the magic for you. Firstly you'll need to understand what relationships are so google around, and when you are done use superb documentation of Laravel** and get all the info from there.
*,**: In early days of Laravel it was the selling point for many developers.
Defining the relationships is hard part of RDBMS design, so take your time, do it right, use pen and paper and then code it - easy part if design is right.
Note: For
user_reactions
use polymorphic relations.
Invest time to learning relationships it will pay off soon :)
Note: I am not writing about how I would define relations you have in code because of lack of context and mainly because of helping too much and I consider it as giving a fish instead of teaching how to fish.
Controller
Routing / Middleware / Route-Model-Binding
Forget about defining middlewares in controller itself, use routes file for that, so delete the __construct()
and just add it back to the routes file. You may ask why, well because of DRY, you will end up with 5-x controllers and each of them will contain $this->middleware('auth');
in constructor, repeating...*
*: Unless you make a "master" class like AuthController and extend from there - IMO that just adds more complications than good if you want to define middleware only.
Use groups in route file, and define middleware on the groups part, see below for route example.
Route Model Binding another super useful feature of Laravel. Example vs bunch of words:
Route:
Route::middleware(['auth'])->group(function ()
Route::get('profile/user', 'ProfileController@index'); <-- Route Model Binding
Route::get('profile/edit', 'ProfileController@edit');
);
Signature of ProfileController@index
method should be index(AppUser $user)
, you see behind the scenes Laravel does query to the database and fetches User
instance for you.
After defining all the relationships on User model you will most likely end up with using a oneliner to get the $user
:
$user = User::with(['photos', 'interests', 'gifts', 'visits', 'likes'])->whereId($id)->get(); // if you stick with not using Route Model Binding
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']); // if you use Route Model Binding
// in view you will access interests for example $user->interests
Now what about edge case if user is looking at their own profile, as I understand the only difference is to change variable $page_name
, do it right in the view (I would do it that way):
@if($user->id == Auth::user()->id)
My Profile
@else
$user->name
@endif
So proper controller after suggestions would look like:
<?php
namespace AppHttpControllers;
use AppUser;
use Auth;
class ProfileController extends Controller
public function index(User $user)
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']);
return view('profile.my_profile', compact('user'));
Packages
Do not reinvent the wheel, for example for handling attachments / images use solution that is tested https://github.com/spatie/laravel-medialibrary and used by many!
During development debugbar is a must.
add a comment |Â
up vote
4
down vote
accepted
First of all do not use Laravel 5.4 even if you've written some* code already, use latest 5.6, and try to always update.
*: You really need to rewrite the application if you have code like you presented everywhere, so just take your time and use L5.6. Idea is already there you just need to improve it - pure coding, no thinking.
Model(s)
Standards
You are sticking to the standard (I guess because User model is the stock one) and using class name User
for a model means that table is plural which renders $table
property useless and you save one line.
Each table shall have its own Model.
Note that we did not tell Eloquent which table to use for our Flight model. By convention, the "snake case", plural name of the class will be used as the table name unless another name is explicitly specified.
Relationships
Main reason you have read about Laravel when you were googling around which framework to use is because its ORM Eloquent*. Eloquent is a abstraction so you do not even need to know how each relationship work under the hood, it does all the magic for you. Firstly you'll need to understand what relationships are so google around, and when you are done use superb documentation of Laravel** and get all the info from there.
*,**: In early days of Laravel it was the selling point for many developers.
Defining the relationships is hard part of RDBMS design, so take your time, do it right, use pen and paper and then code it - easy part if design is right.
Note: For
user_reactions
use polymorphic relations.
Invest time to learning relationships it will pay off soon :)
Note: I am not writing about how I would define relations you have in code because of lack of context and mainly because of helping too much and I consider it as giving a fish instead of teaching how to fish.
Controller
Routing / Middleware / Route-Model-Binding
Forget about defining middlewares in controller itself, use routes file for that, so delete the __construct()
and just add it back to the routes file. You may ask why, well because of DRY, you will end up with 5-x controllers and each of them will contain $this->middleware('auth');
in constructor, repeating...*
*: Unless you make a "master" class like AuthController and extend from there - IMO that just adds more complications than good if you want to define middleware only.
Use groups in route file, and define middleware on the groups part, see below for route example.
Route Model Binding another super useful feature of Laravel. Example vs bunch of words:
Route:
Route::middleware(['auth'])->group(function ()
Route::get('profile/user', 'ProfileController@index'); <-- Route Model Binding
Route::get('profile/edit', 'ProfileController@edit');
);
Signature of ProfileController@index
method should be index(AppUser $user)
, you see behind the scenes Laravel does query to the database and fetches User
instance for you.
After defining all the relationships on User model you will most likely end up with using a oneliner to get the $user
:
$user = User::with(['photos', 'interests', 'gifts', 'visits', 'likes'])->whereId($id)->get(); // if you stick with not using Route Model Binding
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']); // if you use Route Model Binding
// in view you will access interests for example $user->interests
Now what about edge case if user is looking at their own profile, as I understand the only difference is to change variable $page_name
, do it right in the view (I would do it that way):
@if($user->id == Auth::user()->id)
My Profile
@else
$user->name
@endif
So proper controller after suggestions would look like:
<?php
namespace AppHttpControllers;
use AppUser;
use Auth;
class ProfileController extends Controller
public function index(User $user)
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']);
return view('profile.my_profile', compact('user'));
Packages
Do not reinvent the wheel, for example for handling attachments / images use solution that is tested https://github.com/spatie/laravel-medialibrary and used by many!
During development debugbar is a must.
add a comment |Â
up vote
4
down vote
accepted
up vote
4
down vote
accepted
First of all do not use Laravel 5.4 even if you've written some* code already, use latest 5.6, and try to always update.
*: You really need to rewrite the application if you have code like you presented everywhere, so just take your time and use L5.6. Idea is already there you just need to improve it - pure coding, no thinking.
Model(s)
Standards
You are sticking to the standard (I guess because User model is the stock one) and using class name User
for a model means that table is plural which renders $table
property useless and you save one line.
Each table shall have its own Model.
Note that we did not tell Eloquent which table to use for our Flight model. By convention, the "snake case", plural name of the class will be used as the table name unless another name is explicitly specified.
Relationships
Main reason you have read about Laravel when you were googling around which framework to use is because its ORM Eloquent*. Eloquent is a abstraction so you do not even need to know how each relationship work under the hood, it does all the magic for you. Firstly you'll need to understand what relationships are so google around, and when you are done use superb documentation of Laravel** and get all the info from there.
*,**: In early days of Laravel it was the selling point for many developers.
Defining the relationships is hard part of RDBMS design, so take your time, do it right, use pen and paper and then code it - easy part if design is right.
Note: For
user_reactions
use polymorphic relations.
Invest time to learning relationships it will pay off soon :)
Note: I am not writing about how I would define relations you have in code because of lack of context and mainly because of helping too much and I consider it as giving a fish instead of teaching how to fish.
Controller
Routing / Middleware / Route-Model-Binding
Forget about defining middlewares in controller itself, use routes file for that, so delete the __construct()
and just add it back to the routes file. You may ask why, well because of DRY, you will end up with 5-x controllers and each of them will contain $this->middleware('auth');
in constructor, repeating...*
*: Unless you make a "master" class like AuthController and extend from there - IMO that just adds more complications than good if you want to define middleware only.
Use groups in route file, and define middleware on the groups part, see below for route example.
Route Model Binding another super useful feature of Laravel. Example vs bunch of words:
Route:
Route::middleware(['auth'])->group(function ()
Route::get('profile/user', 'ProfileController@index'); <-- Route Model Binding
Route::get('profile/edit', 'ProfileController@edit');
);
Signature of ProfileController@index
method should be index(AppUser $user)
, you see behind the scenes Laravel does query to the database and fetches User
instance for you.
After defining all the relationships on User model you will most likely end up with using a oneliner to get the $user
:
$user = User::with(['photos', 'interests', 'gifts', 'visits', 'likes'])->whereId($id)->get(); // if you stick with not using Route Model Binding
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']); // if you use Route Model Binding
// in view you will access interests for example $user->interests
Now what about edge case if user is looking at their own profile, as I understand the only difference is to change variable $page_name
, do it right in the view (I would do it that way):
@if($user->id == Auth::user()->id)
My Profile
@else
$user->name
@endif
So proper controller after suggestions would look like:
<?php
namespace AppHttpControllers;
use AppUser;
use Auth;
class ProfileController extends Controller
public function index(User $user)
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']);
return view('profile.my_profile', compact('user'));
Packages
Do not reinvent the wheel, for example for handling attachments / images use solution that is tested https://github.com/spatie/laravel-medialibrary and used by many!
During development debugbar is a must.
First of all do not use Laravel 5.4 even if you've written some* code already, use latest 5.6, and try to always update.
*: You really need to rewrite the application if you have code like you presented everywhere, so just take your time and use L5.6. Idea is already there you just need to improve it - pure coding, no thinking.
Model(s)
Standards
You are sticking to the standard (I guess because User model is the stock one) and using class name User
for a model means that table is plural which renders $table
property useless and you save one line.
Each table shall have its own Model.
Note that we did not tell Eloquent which table to use for our Flight model. By convention, the "snake case", plural name of the class will be used as the table name unless another name is explicitly specified.
Relationships
Main reason you have read about Laravel when you were googling around which framework to use is because its ORM Eloquent*. Eloquent is a abstraction so you do not even need to know how each relationship work under the hood, it does all the magic for you. Firstly you'll need to understand what relationships are so google around, and when you are done use superb documentation of Laravel** and get all the info from there.
*,**: In early days of Laravel it was the selling point for many developers.
Defining the relationships is hard part of RDBMS design, so take your time, do it right, use pen and paper and then code it - easy part if design is right.
Note: For
user_reactions
use polymorphic relations.
Invest time to learning relationships it will pay off soon :)
Note: I am not writing about how I would define relations you have in code because of lack of context and mainly because of helping too much and I consider it as giving a fish instead of teaching how to fish.
Controller
Routing / Middleware / Route-Model-Binding
Forget about defining middlewares in controller itself, use routes file for that, so delete the __construct()
and just add it back to the routes file. You may ask why, well because of DRY, you will end up with 5-x controllers and each of them will contain $this->middleware('auth');
in constructor, repeating...*
*: Unless you make a "master" class like AuthController and extend from there - IMO that just adds more complications than good if you want to define middleware only.
Use groups in route file, and define middleware on the groups part, see below for route example.
Route Model Binding another super useful feature of Laravel. Example vs bunch of words:
Route:
Route::middleware(['auth'])->group(function ()
Route::get('profile/user', 'ProfileController@index'); <-- Route Model Binding
Route::get('profile/edit', 'ProfileController@edit');
);
Signature of ProfileController@index
method should be index(AppUser $user)
, you see behind the scenes Laravel does query to the database and fetches User
instance for you.
After defining all the relationships on User model you will most likely end up with using a oneliner to get the $user
:
$user = User::with(['photos', 'interests', 'gifts', 'visits', 'likes'])->whereId($id)->get(); // if you stick with not using Route Model Binding
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']); // if you use Route Model Binding
// in view you will access interests for example $user->interests
Now what about edge case if user is looking at their own profile, as I understand the only difference is to change variable $page_name
, do it right in the view (I would do it that way):
@if($user->id == Auth::user()->id)
My Profile
@else
$user->name
@endif
So proper controller after suggestions would look like:
<?php
namespace AppHttpControllers;
use AppUser;
use Auth;
class ProfileController extends Controller
public function index(User $user)
$user->load(['photos', 'interests', 'gifts', 'visits', 'likes']);
return view('profile.my_profile', compact('user'));
Packages
Do not reinvent the wheel, for example for handling attachments / images use solution that is tested https://github.com/spatie/laravel-medialibrary and used by many!
During development debugbar is a must.
edited May 31 at 9:25
answered May 31 at 9:00
Kyslik
1815
1815
add a comment |Â
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f194343%2flaravel-usercontroller-design%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
2
It looks like you're not using Laravel relationships which reduce a lot of your code in the User model. See here laravel.com/docs/5.6/eloquent-relationships
â Josh Griggs
May 14 at 12:28
What version of Laravel are you using?
â Ross Wilson
May 30 at 7:39
@RossWilson Laravel 5.4
â garjoka madafa
May 30 at 17:44