PSR and Code Style Guidelines

Quick definitions: PHP-FIG is the PHP Interoperability Group, a coalition of individuals representing many of the most popular PHP projects in the world.

What do they do? They tell you each other how to write PHP code such that it’s easier for developers from one project to work on another. It’s the closest thing PHP users have to a governing authority since the creators of PHP seem more focused on creating PHP. I generally see PHP-FIG as a Good Thing™.

PHP-FIG release PSRs. These are PHP Specification Requests — guidelines to how PHP programs should structure their code with interoperability in mind. If you’ve ever used composer you’re familiar with PSR-0, whether you know it or not. That’s the value of a group like PHP-FIG — they create a bedrock for other people to build on.

PSR-2 and Information Density

Symfony, the framework OroCRM is built on top of, hews closely to the PSR standards. These two bits of PSR-2 recently came to my attention

Property names SHOULD NOT be prefixed with a single underscore to indicate protected or private visibility.

Method names SHOULD NOT be prefixed with a single underscore to indicate protected or private visibility.

This bummed me out a little — it’s true that many PHP 4 coding guidelines recommended the use of underscores to signal private/protected visibility. This was necessary in a PHP 4 world where everything was public, and PSR-2’s forcing of these visibility modifiers is the right move. However, there’s still value in having a method’s name signal its visibility level.

One quick example: As part of my first first OroCRM bundle, I’m poking around twig’s internals, trying to get a handle on how the template system actually works behind the scenes. Here’s a bit of code I came across

#File: vendor/twig/twig/lib/Twig/Environment.php
if (!$this->runtimeInitialized) {
    $this->initRuntime();
}

return $this->loadedTemplates[$cls] = new $cls($this);

Without underscores on the runtimeInitialized and loadedTemplates properties, I had no idea if they were protected of public, and therefore no idea if these were properties meant for the developers of twig (system developers), or public properties meant for use by users of twig (client developers).

Yes, I could have stopped what I was doing to look this information up, but that would have meant losing flow in the task at hand. The ability to scan a piece of code and instantly know if something is public or private/protected is valuable when you’re coming up to speed on a new system, or re-familiarizing yourself with an old one.

Also, there’s the idea that a code style guideline should make bad code look bad. When I come across a method that’s heavily relying on public properties without type checking, it looks wrong to me. Code that follows PSR-2 to the letter removes my ability to do this.

Wrap Up

It’s easy to understand why the folks behind PSR-2 wanted those lines in the specification — reenforcing the visibility requirement is a valuable political/evangelism tool. However, while its more a mosquito bite than a death blow to interoperability, removing those underscores removes information density from code, and makes the job of learning a new system just a little bit harder.

Fortunately, PSR-2 uses the infamous language of RFC 2119, which defines SHOULD NOT as

SHOULD NOT This phrase, or the phrase “NOT RECOMMENDED” mean that
there may exist valid reasons in particular circumstances when the
particular behavior is acceptable or even useful, but the full
implications should be understood and the case carefully weighed
before implementing any behavior described with this label.

This means we can, after a careful Anubis like weighing of the pros and cons, continue to use underscores in our property and method names, and get into endless internet arguments about what does and does not follow the spec.