How useful is PHP CodeSniffer? Code Standards Enforcement in General?

user_46675 0
Viewed 38435

I'm dabbling with the idea of setting up PHP CodeSniffer on our continuous integration server in an effort to improve the quality of our code-base. After reading the documentation I'm very excited about the idea of normalizing and enforcing our coding standards. However, I'm left wondering about the actual improvement to our product. I'm well aware that the sniffer only detects violations to a defined coding-standard but what type of benefits does a clean, consistent, code-base provide? Is it worth the extra work to refactor a project with 100k+ lines of code to conform to the PEAR standard?

For those who are not familiar with PHP CodeSniffer or code-smell in general, here is an example output:

FILE: /path/to/code/myfile.php
FOUND 5 ERROR(S) AFFECTING 2 LINE(S)
--
2 | ERROR | Missing file doc comment
20 | ERROR | PHP keywords must be lowercase; expected "false" but found "FALSE"
47 | ERROR | Line not indented correctly; expected 4 spaces but found 1
51 | ERROR | Missing function doc comment
88 | ERROR | Line not indented correctly; expected 9 spaces but found 6

Strictly speaking, the user/client would not notice any difference in a product that was refactored to be standards-compliant but I'm wondering if there are other hidden benefits

Right now our code is by no means sloppy and we try to follow our own personal standards which, for the most part, are derived from Pear's Coding Standards but a trained eye can spot the differences.

So my question is how much do they improve the quality of a product. What kind of latent benefits resulted from it?

Am I just being obsessive-compulsive with my desire to move our product closer to a set of standards? Would it be worth it? If so, What kind of strategy did you use to implement the code-sniffer and correct the subsequent violations that were detected?

codesniffer php
8 Answers

Having coding style conventions is a good idea, because it helps developers not get distracted by code written in a different style when working on code they did not write. It will make your code base superficially cleaner. It's great if you can automate it, but there is usually no need to go through great lengths to comply (unless the current style is terrible). If you already have a good-enough standard, stick to it.

Code smell is something different though: it is (a set of) symptoms that may indicate a deeper problem with the code. Examples are cyclomatic complexity, long method names, large classes, undescriptive names, duplicate code, etc. This is usually much more problematic, as it may thoroughly hurt the maintainability of your code. You should definitely solve these problems.

PHP CodeSniffer appears to be mainly developed for checking style conventions, not code smell. If you can use it to help enforce style conventions, great. But beware that it will not make your code base substantially better. You will want to do manual reviews to accomplish that.

If you want to use it to check if you comply to your current standard, that appears to be possible, see the answer to the question "I don't agree with your coding standards! Can I make PHP_CodeSniffer enforce my own?" in their FAQ.

user_980380

There are lots of cases that require human judgement, and CodeSniffer doesn't have one.

Consistent brackets, indentation improve the code. Lack of space after commas in function call? Probably can be forgiven, but that's ERROR according to CodeSniffer.

IMHO there are way too many errors reported by CS. Even projects that appear to have neat code can easily run into thousands of CS issues. It quickly becomes tiring and nearly impossible to resolve all those issues, especially when it's a mix of real problems and obsesive-compulsive nonsense — both equally often marked as ERRORS.

You may be better off ignoring CS and spending time on actual improvements to the code (in terms of design, algorithms) rather than just completely superficial whitespace and comments changes (does 1-line isAlpha function really need 8 lines of comments? Yes, if you ask CS).

CS can too easily become turd-polishing tool.

user_270090

Are you providing PEAR packages for public distribution thru PEAR/PECL? If so then you probably want to stick to PEAR conventions.

Otherwise, I can't see it being worth the big refactor. The biggest thing is agreeing upon a coding standard for your team... doesn't have to be PEAR's standard... just make sure there is some standard convention enforced.

For instance, I'm a fan of the 

function foo () {

format vs the PEAR standard..

function foo ()
{

Bottom line, don't worry too much about conforming to their standard if its going to be a ton of work, especially if you aren't putting the packages on PECL.

user_1054840
Related