Thanks to a number of open source contributors, I released WP Audio Player 1.3 to the WordPress plugin repository. For one of the pull requests, I mentioned the following:
I always terminate my blocks with a closing comment. Please keep this in the file.
Travis Northcutt also asked me about it via Twitter:
@tommcfarlin curious about always terminating blocks with a closing comment – what's the rationale there?
— Travis Northcutt (@tnorthcutt) January 14, 2013
When it comes to writing code, I try hard to make sure that anything I do that’s outside of the normal coding conventions for a given platform has a rationale behind it.
Case in point: Ending my terminating braces with code comments.
Commenting Code Blocks: Terminating Comments
Simply put, I terminate blocks with code comments simply because it makes finding the matching closing brace to its opening brace much easier regardless of the IDE that the developer is using.
To be clear, I’m don’t particularly subscribe to the arguments for or against any given language. You won’t hear me strongly defending PHP nor will I be found arguing about why an aspect of VB.NET is lame.
It’s not so much that I don’t get said arguments – because I do – but I’m more concerned with working within whatever constraints of the platform I have and trying to do a good job within them not only for myself, but for whatever developers end up working with the code that I’ve written.
In the most simplistic example, let’s say that you have a foreach loop responsible for iterating through the contents of a collection:
foreach( $collection as $item ) {
// Do work...
} // end foreach
This is easy enough to follow that it seems pointless to add a comment to the terminating block. After all, it’s clear where the loop ends, right?
Now, let’s say that the given loop is responsible for setting the current item to -1:
foreach( $collection as $item ) {
$item = -1;
} // end foreach
Still trivial, right?
Now let’s say that we want to set the current item equal to to -1 if and only if the current environment’s foo_condition() is currently true.
foreach( $collection as $item ) {
if( foo_condition() ) {
$item = -1;
} // end if
} // end foreach
Now it’s slightly more complicated, but still easy enough to follow.
But let’s say that foo_condition() isn’t true. And let’s say that there’s a separate collection through which we need to iterate.
And then, during the nested loop, we need to compare the current $item‘s value to the current foo value and, if it’s not the same, then we’ll assign it to the current item.
Even describing it is a little cumbersome, isn’t it?
Anyway, the code would then look something like this:
foreach( $collection as $item ) {
if( foo_condition() ) {
$item = -1;
} else {
while( foo_condition() ) {
if( $item->value != get_foo_value() ) {
$item->value = get_foo_value();
} // end if
} // end while
} // end if
} // end foreach
At this point, the code is relatively simple in that it’s not doing anything complicated, but there’s a lot of nesting going on. It’s not at all unreasonable to assume that a third condition could be introduced this making this particular block of code exceed the height of my IDE’s viewport.
On top of that, let’s say that I’ve pulled code that uses spaces instead of tabs and terminating braces aren’t properly aligned (let alone the rest of the code).
Finally, let’s say that someone pulls my code but their IDE doesn’t allow me to collapse blocks or renders vertical lines from the opening and closing of a block.
If it’s one – or all – of the above, it can be a pain to work with, so adding a single comment to the terminating brace of a code block can go a long way in making the code just a bit easier to read especially in an open source culture.
What About Alternative Syntax?
Of course, PHP offers alternative syntax such that the above code could be written like this:
foreach( $collection as $item ):
if( foo_condition() ):
$item = -1;
else:
while( foo_condition() ):
if( $item->value != get_foo_value() ):
$item->value = get_foo_value();
endif;
endwhile;
endif;
endforeach;
At first glance, this is actually like combining the code comments and the curly braces from above, right? On top of that, it’s also similar to white-space delimited languages like Ruby, Python, and so on.
But this does nothing to address any of the problems above. In fact, I’d argue that this is actually worse because most IDEs won’t match an endif to its opening : at all.
A Word About Legacy Code
In the comments, Gary pointed out that the example given above is actually more of a ‘code smell’ than a best practice for writing code primarily because a lot of the deep nesting can be refactored into their own functions that result in significantly easier code to read and maintain.
As such, I wanted to make sure that I clarify here that I find this practice particularly useful when working with an existing codebase or in legacy code when it’s not always feasible to refactor an entire system.
This is one of those things that’s a small step towards improvement without completely derailing a project into an time suck of oblivion.
That said, it’s still something that I’ve begun to do out of habit if for nothing else to identify where functions and blocks end and to be consistent with how I’ve been commented my LESS files and my front end templates.
So, after all of the above, that’s why I terminate my blocks with comments. I’ve found that it makes reading code just a bit easier to follow regardless of the environment.
I’m a strong advocate for doing this too.
I remember a few years back, I submitted an assignment at university (I think it was Python) using this methodology. The lecturer actually docked me several marks for ‘Inappropriate commenting style.’
I’d love to say that it surprises me, but it doesn’t.
The truth is that when it comes to certain assignments in schools – or even coding conventions in the context of a larger work place – sometimes you have to abide by the rules that are in place.
For what it’s worth, I think the culture of the workplace is easier to change especially if you’re able to demonstrate why changing it will actually improve the quality of work.
Personally I’m a big fan of the alternative syntax. I use PhpStorm as my IDE and it matches everything wonderfully :) I find it’s much faster for me to read and code in alternative syntax. To me it seems redundant that you’re having to leave comments when you could just code instead and ditch the braces. I’ve talked about this with quite a few of my WordPress devs and I’m the weirdo our of them though!
P.S. You can tell that you’re used to using the ‘normal’ syntax rather than the alternative syntax cause you accidentally left a { after the opening foreach in your alternative syntax snippet ;)
Keep up the great work with your blog posts Tom. I always read them but rarely comment. #mybad
Thanks for the heads up on the typo – resolved it as soon as I saw your comment :).
I would say that I’m not advocating for one style over the other, but I clearly am ;). The truth is, whereas phpStorm provides matching for the alternative syntax, other IDE’s do not. So when others go to work with your code, they end up having to trace back and find where the starting semicolon is.
And though I totally get where you’re saying that it’s the best of both worlds, I think that that’s only true if the IDE in question supports the syntax – many of which do not. That’s really where my main complaint about it.
Oh, and thanks for chiming in, too – always dig hearing from those who read ;).
An interesting approach and one I have considered a few times although I have yet to consistently implement it. Although I would also like not use the commenting form you are showing in your examples (the double slashes) in favor of using the “slash-star” (i.e.: /* … */) commenting form.
The reason being is if one (or another) removes all of the white space and the line breaks you will break the code by commenting out (from your example above) all of the closing braces after the deepest closing nested one. For example, in a very highly compressed version of the sample code:
foreach( $collection as $item ) { if( foo_condition() ) { $item = -1; } else { while( foo_condition() ) { if( $item->value != get_foo_value() ) { $item->value = get_foo_value(); } // end if } // end while } // end if } // end foreachOops … sorry about the layout breakage :(
No worries :)
It makes perfect sense to me and I adopted this convention when I saw you use it in your code.
I already used it for div endings in HTML
Yep – I do it for my LESS files as well as my markup especially because markup in templates can be fragmented across many files.
It goes a long way to help in maintenance.
So do I. I often use this for HTML as it doesn’t have
endif, endwhile. It’s quite hard to differentiate closing divs while the HTML code is long (as it always is).But for PHP, I don’t use at all. Same reason as Gary pointed out. With help of IDE, I have no problems with closing brackets. A good practice is writing function or block code as simple (short) as possible or refactoring to achieve that (PIG – PHP Interoperability Group recommends this in their PSRs – PHP Standards Recommendations). If a function or block quote is too long or code is not ours, we can use IDE to automatically reformat it (I use PHPStorm and it has this great future).
I am also using this approach — and just did it as autodidact because I began learning PHP last year and this way of commenting really helped to keep all organized, readable and understandable – for myself :)
I’ve began wrting WordPress plugins and also using this way for writing code – especially for the long nested if statements and so on I can only recommend this to anyone!
Dig this, David.
Don’t miss Gary’s commet below, either. He provides some really good advice on how to handle long, nested statements that are useful especially if you have time to refactor and existing codebase or even begin planning out code from the beginning of a project.
Thank you!
I agree with you and I understand where Gary is coming from – I do not say I like(d) such long if statements – really not – and I am about to avoid where possible.
The suggestion to go with a class and methods here is really what I should go next! Thank you both! :)
I totally have to disagree.
If you’re having to comment closing braces because of deep nesting, this should be a code smell that the code needs refactoring.
Non-trivial conditionals can be pulled out to their own methods, as can the contents of the individual paths. Refactoring should aim to reduce cyclomatic complexity, and provide simpler unit testable methods.
Simpler methods often mean the code is more self-documenting. From your example for instance, the while block could be moved into a method that takes the $item as an arg, and returns $item that may or may not have the value property set, meaning the parent else is replaced with something like $item = $this->maybe_set_value(). That removes two of the nesting layers, even from this demo.
thanks, I had no idea what cyclomatic complexity was, I looked it up and it makes sense if you’re doing test driven development. Need to get on that.
I’ve talked a little bit about clean code before as well as in the comments of the post.
I’ve also talked about unit testing and why shorter methods matter and how far they can go in improving the readability of the code as well as their testability.
When drafting this post, I considered using an example from part of an existing codebase, but it was easier just to come up with a trivial example to demonstrate the point.
All that to say: I definitely get where you’re coming from and I agree about the code smell, but the problem is that in many projects – especially those that already exist – we, as developers, don’t have time to often go back and refactor an entire system to use cleaner code and/or smaller methods.
We do have the ability to make incremental improvements on the code as it stands. Although adding comments to terminating braces is significantly less trivial than breaking the function into smaller methods, it is one step closer to cleaner code. And if that’s the best I can offer for an existing – or even a legacy codebase – then I’ll do it.
But given the chance to do it from the project outset, your approach is spot on and is something that I try to enforce in projects that my team and I work on.
At any rate, fantastic comment – thanks for sharing it, Gary.
That’s a fair response Tom – perhaps the article can have a note added that you find your technique is particularly useful approaching a legacy codebase that doesn’t have the time to be rewritten?
Definitely – I’m also going to be linking to this particular discussion in the comments for others to read, as well.
Like @Bronson Quick, I also use PHPStorm and prefer alternative syntax.
Do you know which IDEs can’t match alternative syntax?
Honestly, I’m not sure.
I’ve been using Coda 2 since it was released (and Coda prior to that). I come from a .NET background and it’s extremely hard to find an IDE that matches the quality of it. Everything else seems less.
I’m actually interested in moving to PHPStorm but I can’t seem to find the time between projects to actually setup and configure the environment for all of my projects. Perhaps over the summer?
At any rate, I don’t know which IDE’s can’t match alternative syntax, though I openly admit I think it’s a matter of preference. At the end of the day, I like the syntax matching and braces more than colons. Just personal opinion, nothing more.