My Top 3 PHP Naming Pet Peeves
I've been writing PHP for quite a while now, but I do not write PHP the same way as when I started. In the beginning, like every developer, I just started by mimicking what other developers did: no surprise there. But along the way, I started to question why I was doing certain things that way and realised that some of the things I did when writing PHP were completely unnecessary.
One of those is naming things. There are a whole lot of assumptions I made when naming stuff. Now I do things differently and have become a bit vocal about it, to the point that I'm actively pushing people to not do these things. It is not that it annoys me -- the title is just clickbait, people! -- but rather that we all should be quicker at honing the most important skill of a software engineer: to question ourselves constantly about why we do things a certain way, and to find better ways of doing them.
And just in case you are wondering, I have way more pet peeves than these three, but these are the ones I could think of related to naming, because:
There are only two hard things in Computer Science: cache invalidation and naming things.
~ Phil Karlton
Interface Suffix
This one is probably one of the ones I don't really understand, but it is so embedded into the PHP community that getting rid of it is going to be almost impossible. I'm talking about the practice of suffixing every interface definition with the word Interface
.
First of all, why? It is not in any PSR or official PHP standard, nor it is explicitly endorsed by anyone I know. It is not that we suffix our classes with the word Class
either. It is just simply done, by everyone.
Some people say that is a practice taken from Java, and I can see that but that does not really explain why Java people started doing it in the first place. Some argue that this is so you can easily distinguish what types are an interface, and I ask, in what context? Your IDE puts different icons on the files accordingly in the file browser. If you are looking at a class and you see implements
next to the type, you know is an interface. And if you are looking at the FQCN in the code, a simple hover tells you what it is. I can understand the practice as valuable in days where IDEs where not a thing. But in this day an age, we have a million ways to know what types are an interface.
But maybe some of you don't like change, and are asking, why not? Well, a strong reason is that those are 9 extra characters that you don't need to type and that don't need to be polluting your source code. Take a look at the definition of the Serializer class of Symfony Serializer (@1e69e2f):
<?php
class Serializer implements SerializerInterface, ContextAwareNormalizerInterface, ContextAwareDenormalizerInterface, ContextAwareEncoderInterface, ContextAwareDecoderInterface
Now, take a look at the simplified definition, without suffixing:
<?php
class Serializer implements Serializer, ContextAwareNormalizer, ContextAwareDenormalizer, ContextAwareEncoder, ContextAwareDecoder
Much shorter huh? We have removed 45 characters from that line, which is a ton considering that is more than half of the recommended characters per line (80) -- not that I agree with that recommendation though. And not only in this source file but on every single file that references the interface -- and interfaces are referenced a lot! -- But the bottom line is that less unnecessary pollution in your source code is always going to be a good thing!
If you are a good observer, you'll realise that my suggestion leaves the code in an incorrect state. We have a name clash. We are defining a class called Serializer
, that implements an interface called exactly the same. You might think this is a drawback, but it is not. You see, one of the benefits of having an interface without a suffix is that now the implementations can describe better what they do. I would rename the Serializer
class to DefaultSerializer
or MainSerializer
to indicate that this is sort of the class that glues the whole library together. This would be the final result:
<?php
class DefaultSerializer implements Serializer, ContextAwareNormalizer, ContextAwareDenormalizer, ContextAwareEncoder, ContextAwareDecoder
Think about this: the interface is an abstract thing, and therefore it makes sense it has the purest, unsuffixed name of what it is representing or abstracting. Concrete implementations should be the ones prefixed with what kind of implementation are they providing and its usually tied to their dependencies. Here are some examples:
Interface | Implementations |
|
|
|
|
|
|
Examples of interfaces names and implementations
I hope this is reason enough for you to start dumping Interface
as a suffix for your interface names. Same goes for Abstract
prefixing abstract classes or Trait
suffixing traits.
Naming after Patterns
Another naming related. Similar to the one before, this refers to the bad practice of suffixing or prefixing class names with the name of the pattern you are implementing in that type. It fills your codebase with class names called LoggerHttpTransportDecorator
, CompositeIdentityProvider
, IdentityProviderAdapter
or EventDispatcherSingleton
. I mean, I know you know your design patterns -- as you should! -- but come on, you don't have to shove it on everyone's face! Do I need to know if that specific class is a decorator, an adapter or a singleton? Not really!
Don't get me wrong: you must know how to identify whether a particular class is implementing a certain pattern just by glancing at it. But you don't need to name them like that. It's just extra unnecessary noise to the source code.
This happens not with just the classical design patterns, but with objects describing their role, like MoneyValueObject
, RestRequestTransfer
, CustomerDTO
or UserEntity
. Just don't do it. You don't need it. The fact that a class is a DTO should have nothing to do with its name. Naming should be short and clear. If the name is ambiguous, then the namespace is there to provide context. Remember that a class name is its full name, not just the last bit after the \
. This brings me to my next point.
Superfluous Namespacing
I've written about this before. Namespaces' role is to prevent name collisions, not create taxonomies. Of course, because of code organisation, you will still have some degree of taxonomisation, but you should still strive to keep things short. There are two common mistakes here. One is using namespaces to describe what things inside them are, and another is abusing them by repeating words or concepts inside them.
The first manifests when you have a namespace called Exceptions
, Interfaces
, Components
, etc. This is a very common way of organising namespaces that people use when working on a project in a framework. I'm more of the opinion that you should use namespaces to group things under the same concern, and not to group things under the same role in a codebase.
For instance, compare these two directory structures:
App/
├── Controllers/
│ ├── UserController.php
│ ├── RoleController.php
│ ├── TokenController.php
│ ├── PaymentController.php
│ └── OrderController.php
├── Entities/
│ ├── User.php
│ ├── Role.php
│ ├── Token.php
│ ├── Payment.php
│ └── Order.php
└── Exceptions/
├── UserNotFound.php
├── TokenNotFound.php
├── ControllerError.php
└── PaymentError.php
App/
├── Security/
│ ├── UserController.php
│ ├── RoleController.php
│ ├── TokenController.php
│ ├── User.php
│ ├── Role.php
│ ├── Token.php
│ ├── UserNotFound.php
│ └── TokenNotFound.php
├── Checkout/
│ ├── PaymentController.php
│ ├── OrderController.php
│ ├── Payment.php
│ ├── Order.php
│ └── PaymentError.php
└── ControllerError.php
The first iteration uses namespaces as a way of categorising by role. In other words, we put all the controllers in a single place, all the entities in a single place, etc. This makes it very hard to determine dependencies and is a very framework-based way of organising namespaces.
The second iteration is categorised by concern. We put all the stuff related to checkout together, and then all the things related to security. Things that are particular to everything, go a level under -- like the controller error. It is much easier to track dependencies here.
Another way of misusing namespaces is to make them too long by repeating stuff in them. Take, for instance, one of my favourite examples from the Laravel codebase: Illuminate\Broadcasting\Broadcasters\Broadcaster
. That is a lot of repetition! This interface could very well have been named Illuminate\Broadcaster
. You don't need all the taxonomy fluff. If a namespace repeats something, then it is superfluous. Once again you must pay attention to how you are breaking down your code.