Code Nuances, Important to Who Hears/Reads

This is a long promised post.

Let’s start.

What is wrong with the following picture?

$subject        = $actionManager->findOrCreateComponent($user);
$someMorelonger = $actionManager->execute($subject);

First, the brain look for differences to understand what is being written. You don’t see a book aligning their similar *Howevers* across pages. What you see is in a poem the font is well taken care and the procedures administered to markup based on syntax.
Aligning equals in this case makes it harder to maintain. Suppose you add a third line, that has a longer property name, now you need to change three lines instead of one. Makes no sense right?

Second, the brain works reading from left to right and from top down. You don’t gain anything by indenting weirdly spaces like this before an equal sign. It is code, poetry, not a senseless computer matrix.

The other example aligned with this mindset is. What happens when the word is too long like here:

    "phpunit/phpunit":                      "~4.1.4",
    "matthiasnoback/symfony-service-definition-validator": "~1.2.0",
    "raulfraile/ladybug": "~1.0.8"

All plot to have neatly aligned versions is thrown to the trash can! Just don’t waste time doing this is my advice.

Let’s see another of this well known hassles:

<?php
 
namespace Bafford\PasswordStrengthBundle\Validator\Constraints;
 
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
 
class PasswordStrengthValidator extends ConstraintValidator
{
    public function validate($value, Constraint $constraint)
    {
        if($value === null)
            $value = '';
 
        if($constraint->minLength > 0 && (strlen($value) < $constraint->minLength))
            $this->context->addViolation($constraint->tooShortMessage, array('{{length}}' => $constraint->minLength));
 
        if($constraint->requireLetters && !preg_match('/\pL/', $value))
            $this->context->addViolation($constraint->missingLettersMessage);
 
        if($constraint->requireCaseDiff && !preg_match('/(\p{Ll}+.*\p{Lu})|(\p{Lu}+.*\p{Ll})/', $value))
            $this->context->addViolation($constraint->requireCaseDiffMessage);
 
        if($constraint->requireNumbers && !preg_match('/\pN/', $value))
            $this->context->addViolation($constraint->missingNumbersMessage);
    }
}

This time in a known password strength bundle. Notice the hassle in maintaining these conditional without braces. It just makes it harder to read and maintain. If one makes a mistake there, hunting the bug could be very tedious and cryptic.

The importance of use for a cs fixer tool cannot be underestimated. For a code that is changing continually, error prone, introducing an error happens at a fast rate. Therefore we should be more careful and integrate the fixer into the cycle.

Picture now simple code turned into a nightmare:

function it_does_the_job();

Into:

/**
 * It does the job.
 * It does some very complicated stuff, but gets the job done.
 *
 * @author Luis Cordova <cordoval@gmail.com>
 * @author Some other author <etc@gmail.com>
 * @package my package
 * @license MIT
 *
 * @param void
 * @return void
 */
public function it_does_the_job();

It just does not make sense when git and services like github et al help us find who did what in a project.

Let’s jump to another nuance, this time is getting into the custom of locking the composer to 3 digits. Some people do:

"jms/di-extra-bundle": "1.4.*",
"jms/aop-bundle": "1.0.*"

Instead of:

"jms/di-extra-bundle": "1.4.1",
"jms/aop-bundle": "1.0.1"

Or similar. The thing is sometimes when they just do 1.4 or ~1.4 some have the wrong idea. You should read this https://igor.io/2013/01/07/composer-versioning.html. However an improvement on top of it is the custom to specify 3 numbers like in:

"jms/di-extra-bundle": "~1.4.1",

This is a better practice since it allows only the minor number to sweep. It does not leave any doubt as to which number is flexible and helps avoiding some undesired scenarios like:

Hm, why 2.2.1 are older then 2.1 ?
I use this format in composer
 
"apy/datagrid-bundle": "~2.1"
And i have 2.2.1 version

:), then you wonder!

There are more nuances but i am shipping this blog to help today rather than later!

7 thoughts on “Code Nuances, Important to Who Hears/Reads

  1. Nice! I keep doing the equal sign align; I agree it makes thing difficult like refactoring, it’s a complete mess. Still it makes it easier for me to read code I wrote ages ago. All in all, excellent post, keep them coming!

  2. I agree with most of them, but the doc comments are still important and @author tags are useful. When you blame a file, you see the latest rewrite of a line, which might be a simple code reformatting or something. Also, I hate seeing a function without explicitly declared visibility 🙂

    • the use case is phpspec, they don’t care about visibility when you spec, because is boilerplate. Regarding the @author tag i still strongly disagree, the blaming can be done in many ways. I believe it hurts the ego of some developers, and that is confronting but I am glad to see that many maintainers receive this change very well.

      • In the GitHub era I don’t think it’s about ego. It’s useful when you know who to contact if you don’t understand something and all. Ego can be measured on the /graphs/contributors page of the project.

        • in good practices you have the author in the composer.json where it needs to be, not in every class making things harder to read and skim over. I don’t think is about ego, it is more about maintaining code.

  3. Ah I finally get the composer versioning problem 🙂

    1.0.x-dev as alias resolves to 1.0.*@dev, but when you have a tag that at least starts with ‘1.0.0’ like a 1.0.0-beta1′ then that version is more PRECISE for the tilde operator, and so the alias is ignored. So you need to use a branch-alias that is higher then the current major/minor-part of the tagged version.

    Using ‘~1.4.1’ will work for end-user projects but for libraries I would rather use something like ‘~1.4,>=1.4.1’ (knowing that the 1.4.1 version does exist).

Leave a Reply

Your email address will not be published. Required fields are marked *