DerekMartin.ca

This is more obvious when creating APIs, but it’s true of all code: Code is an interface. It’s what we interact with to do our jobs. Write code for humans, and let the compiler figure out how to make it fast (or go back and make it faster after it’s working — but always, ALWAYS keep it easy to read & understand!).

The PHP community uses the PSR-1 & PSR-2 coding standards. You can tell PHPStorm to auto-format your code according to those standards. Also, re-read the Laravel Documentation whenever a new version comes out. That’s simple. What follows is a bit more nuanced.

Comments

• If your code is self-explanatory, you shouldn’t need many comments. If you find yourself adding lots of comments, see if you can re-name some variables or methods, remove some levels of indentation, etc, until it reads like plain English.

Annotations

• I’m not a fan of “functional comments” (i.e. annotations), because they break the mental model of what docblocks are, and are for. It’s fine for them to provide hints to your IDE, or for auto-generating documentation, but they should not be used to provide actual functionality (e.g. marking a particular method as a test method).

Plain Old HTML

• attributes are supposed to be surrounded by double-quotes, not singles. In php, this means you should enclose html in single quotes. When you do that, it doesn’t parse variables inside the single quotes. You need to end the quote first, like:

  echo ‘link text’;

• alternately, you can escape the html double-quotes and brace-bracket the variable, like:

  echo “link text’;

• but for the love of God, don’t single quote your attributes, or even, heaven forbid, skip the quotes altogether! This is horrible:

  echo “link text”;

Naming

• Always go for a long descriptive name over a short one that’s quick to type. Your IDE should auto-complete them for you, anyway.
• Avoid having duplicate terms for same thing: home, house, building? Gateway_user_id, user_gateway_id. For more info, look at Domain Driven Design’s concept of “bounded contexts”.
• There’s no need to name things $whateverObj. Everything should be an object. It’s assumed. If anything, use special naming for arrays, which should be fairly uncommon (opt for Objects & Collections instead).
• URLs should map directly to controller & method names. Breaking that convention is ok if it enables re-use, but not “just because”. This makes tracing bug screenshots back to the relevant code much easier, many times a week.
• URLs should be RESTful, meaning GET /users/ returns a list of all users, while POST /users/ creates a user, and GET /users/23 returns user 23, and PUT /users/23 updates user 23.
• Controller and table names should be plural. Model names should be singular. Join tables should refer to the joined tables in alphabetical order, and each should be singular. For example, a table joining users and activities would be called: activity_user
• Avoid names like $data. What KIND of data? What’s it for? Can we pass or return an actual object instead?
• Avoid names like “prepareData”. What data? How are you preparing it? What are you going to do with it?
• Avoid names like Helper. How does it help? What’s it doing, exactly? If it’s doing a bunch of stuff, it should be separate classes. If it is presenting something, call it a Presenter. If it is transforming something, call it a Transformer. With objects & namespaces there’s no reason to put the object name in the method name:

.

api->apiRequest()

vs

api->request()

or

Customer.php->getPaymentGatewayCustomer()

vs

Customer.php->get()

Namespaces

Put the namespace in a use statement to reduce visual clutter in the code:

$details = \App\Models\UserBillingDetails::where(...)

vs

use App\Models\UserBillingDetails;
...
$details = UserBillingDetails::where(...)

If you want a more specific name than the class name, rename the class, or alias it:

use App\Models\UserBillingDetails as UserBillingDetailsModel;

Objects

• You should be asking Objects to tell you about themselves, not inspecting the Object yourself. That’s rude! You don’t look inside someone’s tummy to see if they’re full. You ask them!

Before:

if(empty($object))

After:

if($object->isEmpty())

• Use Objects, not arrays. You can’t enforce anything on arrays.

Before:

$this->membership['expires'] = date('Y-m-d', $event->data->object->current_period_end);
$subscription->sendFail($this->membership, $source);

After:

$membership->expire();

• An object or method should never access anything outside itself directly, like $_SESSION, or DOM elements in JS functions. The object shouldn’t need to know about the structure of data outside itself. If you need to access outside data, pass it into the constructor, then call getters on that object. This simplifies testing, because it allows us to pass a fake or mock into the constructor, instead of the real thing. That makes it easier to isolate what you’re testing.
• The parameters you pass into an object help explain what it’s doing, so pass in the parameters you directly need, not ones you indirectly need.

Before:

function methodName(Account $account){
    $card = $account->getCard();
    return $card->getTotal();
}

After:

function methodName(Card $card){
    return $card->getTotal();   
}

Constructors

• Pass in any objects you’ll need access to, then assign them to properties. That’s dependency injection. It makes testing easier. An especially common thing people do is directly access the session from within an object. You should be passing it into the constructor, so you can swap it out for a fake session in automated tests. As another example, let’s say a Cart stores its items in Redis, but you want to be able to test it without Redis, by storing test data in a simple array instead:

Production:

$cart = new Cart(new RedisStore());

Testing:

$cart = new Cart(new ArrayStore());

Methods & Functions

• Ending a function with “return;” should be very rare. Return a value!

• Specify the Return Type, if you know what it’ll be.

  public function getAccount() : Account

• Don’t echo in a method. Return what you want to return and have the thing that receives it echo it. If you really want a method to output directly, create 2 methods. One that gets the output, and another that outputs it. That way the output data is available to other method, to be output in different ways.

• If you always need to json_decode the return value you get back from a method, return it already json_decoded.
• If you need to return mixed types, try to stick to: Array+Boolean+NULL, or JSON+Boolean+NULL, or Object+Boolean+NULL
• If you need a value in many methods, pass it into the constructor, then access it via $this->varName
• If you only need a value in 1 method, make it a parameter on that method, don’t pass it into the constructor.
• Don’t pass in an array if you only need 1 or 2 values from it.

The function shouldn’t need to know about the structure of data outside itself. Instead, make those values specific parameters. It’s less error-prone, easier to follow, and allows your IDE to auto-complete.

function name(Account $account){ 
    $card = $account->getCard();    
}

vs

function name(Card $card){ 
    ...
}   

Static Methods

• Avoid them as much as possible, because they usually don’t take advantage of $this, which results in more parameters than you need, and in object state not being used… which just creates more work. If you’re using a lot of statics, you’re probably using objects as related groups of functions, and that’s not real Object Orientation.
• Follow Laravel’s lead and use Facades instead of Static classes.

Controllers

• Instead of returning [‘status’=>’ok’], send the appropriate HTTP response code: 200, 400, 404, 500, etc.
• With Laravel, if a public controller method or Route returns an array, it’ll automatically json_encode it. You only need to return response()->json if you want to send an http response code other than 200.
• Objects throw Exceptions, Controllers catch them and redirect appropriately. Flow control is controller’s job/responsibility. What does a controller control? Flow.
• Controllers shouldn’t usually contain protected or private methods (unless it is a BaseController). Put reusable code in models or libraries. Controllers are for flow logic, not business logic.
• Instead of doing queries in Controllers, put them in a method on the object you’re querying! Tidier & more reusable.

Before:

$setting = \App\Models\GatewaySetting::where('site', $source)->first();
$base_uri = $setting->value;

After:

GatewaySetting->getBaseUrl($source);

Models

• A model does not need to have a corresponding database table. It just needs to be a distinct unit of logic. It may just manipulate data, or work with an API, or whatever.
• Try to avoid doing DB::table queries. Instead, use Eloquent’s methods. MANY are built-in. Save yourself some work.
• For Laravel, read the documentation. It’s amazing how much functionality is built-in.
• If you find yourself repeatedly writing queries that have “where site = %s”, try adding a Scope to your model, so you can just say: $model->site($site). It saves a bit of typing and is less error prone.

Sessions

• Sessions are an HTTP thing. They should only be used/referenced in Controllers. Objects should receive them via constructor, or setter. Models should not be tied to session, because then you can’t use them offline, such as on the CLI, cron jobs, or queued jobs. Pass the state you need into the model, or set it on the model. Sometimes it can be a session, other times it can be an object or collection, as long as they obey the same Interface. Note: APIs, and the code inside them, should not use sessions at all. They are inherently stateless. Microservices are just APIs. If they’re using sessions, you’re doing it wrong.

Traits

• There are 2 kinds of “use” statement. Outside codeblock in the header is for activating a namespace. Inside a codeblock before the constructor is for importing methods & properties from Traits.
• Traits aren’t for sharing methods wherever you want. They’re for horizontal inheritance, which is otherwise impossible in PHP. It should make sense for the object using the Trait to have those methods. If not, simply instantiate an object that logically has those methods, and use that object inside the object you’re working on. Traits basically just make the compiler copy paste methods & properties into the classes that use them. Because it’s pasting, Traits shouldn’t include methods or properties that already exist in the objects that’ll use that Trait. It will be flagged as duplicate code, and the compiler will throw an exception.

Views

• If you see a lot of conditionals and logic in a View, move it into the Model, and have the Controller get the result from the Model and push it into the View. For example, say you want to include a formatted price in the view. Instead of formatting it in the view, your Model could add a value to its collection called $collection->formatted_price, and you just pass that into the View. I’ve also seen that technique used to specify the class that should be on an item, like $collection->row_class=’danger’;

Miscellaneous

• If something feels super-tricky, there’s probably an easier way. Ask around.
• Never use extract(). It’s too non-obvious as to what variables it creates, and can lead to overwriting variables you didn’t mean to overwrite. This leads to errors, and unused variables, OR trying to use variables that don’t exist. It also makes your IDE freak out, highlighting the uses of the extracted variables as being undefined.
• Don’t pass an array when you can pass a collection or an object. Use collections whenever possible, not arrays. They have lots of helper methods. Laravel has an excellent Collection class, and there’s a javascript port called collect.js. Making a collection in Laravel is simple:

.

$yourCollection = collect($yourArray);