Phspark Coding Style

Every company follows a different coding standard based on their best practices. Coding standard is required because there may be many developers working on different modules so if they will start inventing their own standards then source will become very un-manageable and it will become difficult to maintain that source code in future.

Table of Contents

• Follow Phspark naming conventions
• Use shorter and more readable syntax where possible
• Single responsibility principle
• Fat models, skinny controllers
• Validation
• Business logic should be in service class
• Don’t repeat yourself (DRY)
• Prefer to use Eloquent over using Query Builder and raw SQL queries. Prefer collections over arrays
• Mass assignment
• Do not execute queries in Blade templates and use eager loading (N + 1 problem)
• Comment your code, but prefer descriptive method and variable names over comments
• Do not put JS and CSS in Blade templates and do not put any HTML in PHP classes
• Use config and language files, constants instead of text in the code
• Use standard Phspark tools accepted by community
• Use IoC container or facades instead of new Class
• Do not get data from the ".env"-file directly
• Store dates in the standard format. Use accessors and mutators to modify date format
• Other good practices
• References

* Code MUST use an indent of 4 spaces for each indent level, and MUST NOT use tabs for indenting.

Follow Phspark naming conventions

Follow PSR standards.

Also, follow naming conventions accepted by Phspark community:

Method naming conventions in controllers

These should follow the same rules as model methods. I.e. camelCase (first character lowercase). In addition, for normal CRUD operations, they should use one of the following method names. Verb URI Typical Method Name Route Name

Use shorter and more readable syntax where possible

Bad:

                                 
$request->session()->get('cart');
   $request->input('name');
                                 

Good:

                                    
session('cart');
   $request->name;
                                 

More examples:

Back to contents

Single responsibility principle

A class and a method should have only one responsibility.

Bad:

                                       
public function getFullNameAttribute()
{
  if (auth()->user() && auth()->user()->hasRole('client') && auth()->user()->isVerified()) {
      return 'Mr. ' . $this->firstname . ' ' . $this->middle_name . ' ' . $this->lastname;
  } else {
      return $this->firstname[0] . '. ' . $this->lastname;
  }
}
                                    

Good:

                                       
public function getFullNameAttribute()
{
  return $this->isVerifiedClient() ? $this->getFullNameLong() : $this->getFullNameShort();
}

public function isVerifiedClient()
{
  return auth()->user() && auth()->user()->hasRole('client') && auth()->user()->isVerified();
}

public function getFullNameLong()
{
  return 'Mr. ' . $this->firstname . ' ' . $this->middle_name . ' ' . $this->lastname;
}

public function getFullNameShort()
{
  return $this->firstname[0] . '. ' . $this->lastname;
}
                                    

Back to contents

Fat models, skinny controllers

Put all DB related logic into Eloquent models or into Repository classes if you’re using Query Builder or raw SQL queries.

Bad:

                                          
public function index()
{
  $clients = Client::verified()
      ->with(['orders' => function ($q) {
          $q->where('created_at', '>', Carbon::today()->subWeek());
      }])
      ->get();

  return view('index', ['clients' => $clients]);
}
                                       

Good:

                                          
public function index()
{
  return view('index', ['clients' => $this->client->getWithNewOrders()]);
}

class Client extends Model
{
  public function getWithNewOrders()
  {
      return $this->verified()
          ->with(['orders' => function ($q) {
              $q->where('created_at', '>', Carbon::today()->subWeek());
          }])
          ->get();
  }
}
                                       

Back to contents

Validation

Move validation from controllers to Request classes.

Bad:

                                             
public function store(Request $request)
{
  $request->validate([

      'title' => 'required|unique:posts|max:255',

      'body' => 'required',

      'publish_at' => 'nullable|date',

  ]);

  ....

}
                                          

Good:

                                             
public function store(PostRequest $request)
{
  ....
}

class PostRequest extends Request
{
  public function rules()
  {
      return [
          'title' => 'required|unique:posts|max:255',
          'body' => 'required',
          'publish_at' => 'nullable|date',
      ];
  }
}
                                          

Back to contents

Business logic should be in service class

A controller must have only one responsibility, so move business logic from controllers to service classes.

Bad:

                                                
public function store(Request $request)
{
  if ($request->hasFile('image')) {
      $request->file('image')->move(public_path('images') . 'temp');
  }

  ....
}
                                             

Good:

                                                
public function store(Request $request)
{
  $this->articleService->handleUploadedImage($request->file('image'));

  ....
}

class ArticleService
{
  public function handleUploadedImage($image)
  {
      if (!is_null($image)) {
          $image->move(public_path('images') . 'temp');
      }
  }
}
                                             

Back to contents

Don’t repeat yourself (DRY)

Reuse code when you can. SRP is helping you to avoid duplication. Also, reuse Blade templates, use Eloquent scopes etc.

Bad:

                                                   
public function getActive()
{
  return $this->where('verified', 1)->whereNotNull('deleted_at')->get();
}

public function getArticles()
{
  return $this->whereHas('user', function ($q) {
          $q->where('verified', 1)->whereNotNull('deleted_at');
      })->get();
}
                                                

Good:

                                                   
public function scopeActive($q)
{
  return $q->where('verified', 1)->whereNotNull('deleted_at');
}

public function getActive()
{
  return $this->active()->get();
}

public function getArticles()
{
  return $this->whereHas('user', function ($q) {
          $q->active();
      })->get();
}
                                                

Back to contents

Mass assignment

Bad:

                                    
$article = new Article;
$article->title = $request->title;
$article->content = $request->content;
$article->verified = $request->verified;
// Add category to article
$article->category_id = $category->id;
$article->save();
                                 

Good:

                                    
$category->article()->create($request->all());
                                 

Back to contents

Do not execute queries in Blade templates and use eager loading (N + 1 problem)

Bad (for 100 users, 101 DB queries will be executed):

@foreach (User::all() as $user)
 
@endforeach
                              

Good (for 100 users, 2 DB queries will be executed):

                                    
$users = User::with('profile')->get();

...

@foreach ($users as $user)

@endforeach
                                 

Back to contents

Comment your code, but prefer descriptive method and variable names over comments

Bad:

                                    
if (count((array) $builder->getQuery()->joins) > 0)
                                 

Better:

                                                               
// Determine if there are any joins.
if (count((array) $builder->getQuery()->joins) > 0)
                                                            

Good:

                                                               
if ($this->hasJoins())
                                                            

Back to contents

Do not put JS and CSS in Blade templates and do not put any HTML in PHP classes

Bad:

                                                                  
let article = ``;
                                                               

Better:

                                                                  
<input id="article" type="hidden" value="">

 Or

<button class="js-fav-article" data-article=""><button>
                                                               

In a Javascript file:

                                                                  
let article = $('#article').val();
                                                               

The best way is to use specialized PHP to JS package to transfer the data.

Back to contents

Use config and language files, constants instead of text in the code

Bad:

                                                                     
public function isNormal()
{
  return $article->type === 'normal';
}

return back()->with('message', 'Your article has been added!');
                                                                  

Good:

                                                                     
public function isNormal()
{
  return $article->type === Article::TYPE_NORMAL;
}

return back()->with('message', __('app.article_added'));
                                                                  

Back to contents

Use standard Phspark tools accepted by community

Prefer to use built-in Phspark functionality and community packages instead of using 3rd party packages and tools. Any developer who will work with your app in the future will need to learn new tools. Also, chances to get help from the Phspark community are significantly lower when you’re using a 3rd party package or tool. Do not make your client pay for that.

Back to contents

Use IoC container or facades instead of new Class

new Class syntax creates tight coupling between classes and complicates testing. Use IoC container or facades instead.

Bad:

                                 
$user = new User;
$user->create($request->all());
                                 

Good:

                                 
public function __construct(User $user)
{
  $this->user = $user;
}

 ....

$this->user->create($request->all());
                                 

Back to contents

Do not get data from the ".env"-file directly

Pass the data to config files instead and then use the config() helper function to use the data in an application.

Bad:

                                    
$apiKey = env('API_KEY');
                                    

Good:

                                    
// config/api.php
'key' => env('API_KEY'),

// Use the data
$apiKey = config('api.key');
                                    

Back to contents

Other good practices

Never put any logic in routes files.

Minimize usage of vanilla PHP in Blade templates.

Back to contents

References

• Coding Standards [https://www.php-fig.org/psr/psr-12]
• Autoloading Standard [https://www.php-fig.org/psr/psr-4]

Back to contents