How important is code formatting?

PHP
1

Recently I tried to create an extension for Drupal 8. I actually got the thing done, then when it came time for code review I was told to get the thing formatted correctly and given links to code format programs to help with the process. So I downloaded and installed them, and found that they didn’t follow the published guidelines. Extremely, finicky crap like whether to format a conditional like this

<?php if(condition) ?>

or this…

<?php if( condition ) ?>

After about an hour of wrestling with it and not even getting through a singly 87 line file to the satisfaction of the software I gave up. I have far, far better things to do with my time.

I understand the importance of consistency and legibility - I’ve only studied typography formally for 6 years and work in the graphics arts field where a grasp of such things is mandatory. But my experience with the drupal team has left a very, very bad taste in my mouth - asking for code formatting precision down to a space count just feels like a college fraternity hazing process and not something approaching professional or necessary.

And it’s very disheartening and saddening to tell the truth.

2

I like the approach that Golang has taken where their compiler reformats the script.

3

I have the luxury of usually only writing my own code by myself for my own use.
So I can write it as I wish.

But when I have attempted to contribute code to existing multi-dev projects I’ve found that they do have “standards” - standard within that particular project only it seems.

Things like

Tabs vs. spaces for indentation

function whatever() {
vs.
function whatever() {

( $a < $b )
vs.
($a<$b)

$variable = 'value'
vs.
'value' = $variable

IMHO the main thing is to be consistent so that it’s easier for you to see what’s going on.

But I guess in some circumstances a “standard” needs to be adhered to whether or not it’s our standard.

Seems that last I used NetBeans it had an “auto-format” thing. Might be easier to run your code through something like that instead of going through it line by line.

4

There is a semi-standard or guide for PHP code style and you’ll notice nothing like spacing in if-statements is mentioned. I am not sure why the code formatter you were using was telling you that. It is totally a personal preference.

For instance, I like lines before and after code blocks, so it is sets them apart. Like so.

<?php
namespace Vendor\Package;

use FooInterface;
use BarClass as Bar;
use OtherVendor\OtherPackage\BazClass;

class Foo extends Bar implements FooInterface
{
    public function sampleFunction($a, $b = null)
    {
        if ($a === $b) {                

            bar();

        } elseif ($a > $b) {

            $foo->bar($arg1);

        } else {

            BazClass::bar($arg2, $arg3);

        }
    }

    final public static function bar()
    {

        // method body

    }
}

It looks a bit silly with just one line of code in the block, but add several lines, and it just makes it more readable. Well, at least for me it does and it is totally a personal preference. Oh, and BTW, Drupal is a member of PHP-FIG. :wink:

Scott

5

If you work in any kind of team environment, and this includes contributing to open source where others may be subjected to your code, coding standards are extremely important.

PSR-1 and PSR-2 are those the PHP-FIG agreed on, and they help with compatibility across projects. Not only in terms of being readable by many people at first glance, but also because they minimize conflicts in version control systems. If you use tabs, and I use spaces, and we commit to the same file after I apply my standard, there will be conflicts on a line that shouldn’t logically conflict. The logic will stay the same, and the conflict will be purely cosmetic. If you put spaces around a condition and I don’t, and I apply my formatting which changes your code to apply to this, this is again a cosmetic conflict. This wastes everyone’s time and effort.

You don’t need to manually adjust your code to fit the standard rules. There are things like CSFixer which take care of this for you and have both a command line interface for manual or task-based launching and support for modern IDEs like PhpStorm, Netbeans, even Eclipse which make sure to warn you of formatting mistakes as you type your code. PhpStorm, for example, not only warns me when I’ve failed to respect a standard but also offers a convenient shortcut for automatically fixing it - I just hit CTRL+ALT+L and the code falls into place.

That said, if you don’t work in a team environment, don’t intend to share your code with anyone, or are in control of a project and can dictate what it uses, you are in no way forced to follow a standard. However, I still recommend it, purely because it makes your code compatible with the eyes of many developers from the get-go, without much sacrifice.

In your particular experience with Drupal, they provide coding standards beforehand so configuring an IDE to automatically apply them as you type is very easy. Now, while Drupal is, admittedly, far from a perfect codebase, their rules are their rules and it’s up to you to respect them if you want to tag along.

6

I was just about to ask, what standards does Drupal have and decided to let Google have a chance at answering my question and low and behold, look at what I found.

https://www.drupal.org/coding-standards#controlstruct

Scott

7

I’ve said this in the past but it bears repeating. Any time spent formatting code manually to meet some predetermined standard is, in my opinion a waste of time. A developers skill is not in how they format the code, it’s the abstract thought process behind the design of the program and making the code work. Using a developers valuable time on something so trivial is a waste of their talent.

My proposed solution? If it’s important for a given project then a simple pre-commit git hook that runs the code through a formatter. The individual developer can run the code through a formatter themselves to work with it in any format they like when they pull from the project. Most IDEs have an option to format the code on open anyway so this could be 100% transparent.

I’m of the opinion that none of this should matter at all.

1 Like
8

The problem isn’t the standards, the problem is the standards they have published and the standards their endorsed formatting software enforces do not match

Which one do I turn in? It’s going to fail one of the two tests guaranteed. That’s unprofessional hazing, no more or less.

9

One thing the Drupal community is they are very elitist about these types of things which is good and bad at the same time. Good in respect to things being standardized but bad in respect to higher learning curve. frustration for new comers to the community. In comparison to an ecosystem like Magento where any and all crap flyes without ANY oversight which makes the entire ecosystem a gigantic mess. I personally rather have oversight, and strict guidelines in place rather than everyone doing things their own way.

10

It’s still a bear when you have mild dyslexia. I lose enough productive time during the day fixing number and letter sequence shifts, dealing with spaces gives me migraines.

11

Regardless, the oversight, and strict formatting makes sense for code which would be added to core. Core should act as the example by which all else follows.

12

Writing code, like writing human languages, does require some conformity to standards. I’ve been coding for a long time and PHP is the first language I’ve used where I thought the formatting “standards” were nuts. I just found that style of PHP very difficult to read, with huge amounts of white space.

So I ignored all that advice and write PHP in a style I prefer. My code is a bit more compact as a result. My friends who have coded for many years can read my code relatively smoothly even if they aren’t PHP programmers. And that is my real goal. When I get hit by the proverbial bus, I want the next coder to at least be able to read the code even if they aren’t crazy about my style.

The last thing I want is my IDE telling me how to code.

13

Yes, I have more trouble reading poorly named functions and variables that I have dealing with spacing or curly braces

14

Although we have big displays these days, they aren’t getting taller, they’re getting wider. The spacing and curly brace positioning that takes up this valuable vertical space is the big thing I don’t like. I believe in white space, I was trained to use white space, but too much white space is wasted space.

15

You can turn a monitor on its side and use it that way. :smile:

I agree. Too much white space is also not good. Like separating methods with more than one line. If you are documenting methods like you should be, the comments are enough to make the “split” between methods obvious and offers enough white space too. Trying to read a bunch of cluttered code without comments is what gives me migraines. LOL! :smiley:

Scott

1 Like
16

It’s so nice to see someone say something like this.

I’ve heard WAY too many times that “well developed code is self-documenting, so we don’t need to put in comments” from the zealots. So instead we have miles of spaghetti code or overloaded methods to handle a new situation because going back, they can’t remember why a method was built the way it was, so they’ll overload it to handle a new situation and not risk changing something else :grimacing:

Even one line comments, especially on “special case” lines of code, save so much work (or re-work) in the long run, it’s not even funny.

Back to the original post, I often find guidelines like this to be onerous, and are often unecessary, and distracting to the real work that needs to be done. Naming conventions (variable names, table names, method structures) are often more advantageous and productive than formatting choices.

Your rules should be simple (and not necessarily in this order) and finite:

  1. Is it readable? Can someone read it and tell what it does?
  2. Is is documented? Especially if there is special logic which applies to limited scenarios…
  3. Is it efficient?
3 Likes
17

One annoying trend in the opposite direction to no comments is autogenerated docblocks that add literally nothing that isn’t in the method header. Some people seem to think repetition = commenting, which is imho worse than omitting the comments entirely.

2 Likes
18

Agreed. Whoever created the autodoc generator concept should be taken back behind the proverbial shed for some good old fashioned discipline :facepunch: :facepunch: :facepunch:

3 Likes
19

Commenting should tell the coder or other coders what that block of code does, so that if a person(s) look at a future date they can figure it out easier. However, commenting code that is obvious in what it does is a waste of time and energy.

20

As long as it’s obvious to everyone…if it’s just obvious to you, then it’s not a waste of time (and how long does it really take to write a comment, 10 seconds?)

3 Likes