PHP Coding Standards

Hi all,

Where I work, we’ve recently started looking at PHP CodeSniffer. And consequently the following rulesets are being advocated:

  1. Squiz.Arrays.ArrayDeclaration
  2. Squiz.Strings.DoubleQuoteUsage
  3. Squiz.Commenting.ClosingDeclarationComment
  4. Squiz.Commenting.LongConditionClosingComment
  5. Squiz.WhiteSpace.FunctionClosingBraceSpace.SpacingBeforeClose

The Array Declaration stipulates that any array with more than one element should be written on multiple lines regardless of whether it is an associative array or not.

$a = array(1,
           2,
           3,
);

//and

$a = array(1 => 'a',
           2 => 'b',
           3 => 'c',
);

The double quote usage, throws an error even when its usage is to enclose a SQL statement that has single quotes.

$sql = 'SELECT * FROM member WHERE lastname='doe' "

The closing declaration comment requires all methods to end with an ‘end comment’ that specifies the method name

class A
{


  public function getLatestResult($name)
  {
  }//end getLatestResult()

}//end class

The spacing before closing statement

class A
{


  public function doSomething($n)
  {

      $x = $n*$n;

  }//end doSomething()


}//end class

All of the above are completely new rules that are to be strictly enforced.

I was wondering what your thoughts are.

As long as you’re consistent it doesnt matter. Most decent IDEs can format the code to the viewers preference anyway.

The only thing I don’t like is the //end comments. They’re entirely redundant imho, most good code has short methods and if you’re using one class per file then signifying the end of the class is also pointless.

Other than that, singe vs double quotes, for consistency I prefer to just use single quotes for everything (pretty much).

Can we have a “None of the above” option?

It is a good thing to be consistent with your code style, however, I wouldn’t say there is a requirement to force a style. It can be very hard, everyone develops their own style.

As has already been said, IDE’s help a lot with code formatting. They can even close blocks of code down out of the way, so the //end comments are a bit surplus to requirements.

Yup, added and voted for it.
All the options make my stomach turn :sick:

Heh heh, yup. Thanks ScallioTX

Legibility should be the concern of any coding standard. After all, the computer doesn’t care if you never touch the enter key. And I’ve seen code laid out that way… blech.

First rule, the major two coding conventions are open or closed brace styles. I prefer closed brace. Here they are


# Closed or "inline"
if ( condition ) {
  //action 1
} else if ( condition ) {
  //action 2
} else {
  //action 3
}

# Open braces
if ( condition ) 
{
  //action 1
} 
else if ( condition ) 
{
  //action 2
} 
else 
{
  //action 3
}

My preference is driven by the fact that the vertical space of inline is much tighter and makes the code easier to parse, at least on a thoughts level. Note that both styles use tabbing, and that’s the key - levels should be tabbed inward. This is so important that at least one major programming language - python - uses tabbing in lieu of braces to mark programming block bodies.

Now for some contrast, here are my rules for things you touched.

Arrays…


# When the key isn't important, just stay on one line
$var = array ( 1, 2, 3, 4, 5 );
$var = array ('apple', 'orange', 'pineapple', 'grape');

# But if it is long enough, indent the second line and close
# the statement with ); by itself.
$var = array ('cat', 'dog', 'goat', 'sheep', 'cow', 'pig',
    'deer', 'snake', 'owl', 'eagle', 'chicken'
);

# If keys are significant, then one per line unless the list is *really*
# short. Judgement call.
$var = array( 1 => 'January', 2 => 'February', 3 => 'March' );

# If you do nest, all elements indented and on same line for that
# level.  This is critical for multilevel arrays.
$var = array(
  'animals' => array (
    'farm' => array (
      'sheep' => 20,
      'dog' => 1,
      'pigs' => 17,
      'cows' => 125
    ),
    'wild' => array (
      'deer' => 1204
    )
  ),
  'plants' => array (
    'farm' => array (
      'corn' => 21000
  )
)

I don’t know what you mean by your double quote stuff. SQL statements in my code have the quotes on their own line if the SQL statment is long

I do enclose all vars in a double quote block with braces to make them stand out.


# short statement
$result = $db->quickQuery("SELECT name FROM people WHERE id = {$id}");

# long statement
$result = $db->quickQuery("
  SELECT name,
    title,
    address
  FROM people
    JOIN address ON people.addressid = address.id
  WHERE age > {$age}
  ORDER BY age DESC
  LIMIT 0, {$recordCount}
");

Adding closing comments can only POSSIBLY be necessary if you are inconsistent with your tabs.

Now some of mine. First, function arguments, if there are enough of them, should be one per line. When to switch from one format to the other is again a judgement call.

 
protected function doSomething (
  ClassA $a,
  ClassB $b, 
  array $c = array()
){
  // function body
}

I’m wary of any function that needs so many arguments that this formatting is necessary and try to think of a way to avoid it, but sometimes it can’t be helped, especially with class constructors whose factories need to pass them multiple dependencies.

Complex if statement blocks can go multiline as well, but this is RARE.


if ( ($a > 3 && $b < 4 && $c == 5) ||
  ($a > 5 && $b < -2 && $c == 7 ) ||
  myfunc($a, $b, $c)
) {
  // body
}

To summarize, formatting is only important to humans and not computers. As such they are guidelines, not hard and fast rules. Bend them, even break them - in the name of making the code as easy to read as possible.

Hi…

Don’t do this. Please. Really, don’t do this. Stop now.

Code is for humans to read. We are contextual, we can only hold five things at once, we are all different, vision is a complicated poorly understood thing and comprehension is even worse. We have monkey brains, not parsers.

Look at the code. Ask yourself, is it readable? Not sure? Grab someone passing by and ask their opinion. If people can ead it, job done. No other criteria matters.

Humans work mostly by chunking so as not to overflow the 5-7 things rule. If you make stuff look flat, you cut accross that. Graphic designers spend a lot of time tuning white space on pages so that like stuff is grouped together and headings and sections stand out. Guess what? All their pages are different.

By any graphic design standard, all of these coding suggestions look pretty deranged to me.

Here is some typical code:


<?php
$month_length = array('jan' => 31, 'feb' => 28, 'mar' => 31,
                      'apr' => 30, 'may' => 31, 'jun' => 30,
                      'jul' => 31, 'aug' => 31, 'sep' => 30,
                      'oct' => 31, 'nov' => 30, 'dec' => 31);
$days = array('mon', 'tue', 'wed', 'thu', 'fri', 'sat', 'sun');

class Calendar { ...
    ...
}
?>

I’m using two different styles here. Here are all the things I’ve thought about:

  1. 12 months won’t fit on one line and would give 24 things to take in. 3 columns seemed a good compromise.
  2. I can clearly see the 30/31 alternation if I use 4 columns. I think I should have done.
  3. I managed to get it all in 5 lines, thus allowing visitors to the file to see the top level Calendar class straight away. That’s the real point of this code. Being economical with vertical space on detail stuff allows developers to see the real meat quickly.

Note that using horizontal whitespace is incredibly tedious to type. I’d avoid using horizintal spacing unless you are really stuck.

Your coding standard wastes valuable vertical whitespace all over the place. Your standards, all of them, make the code less readable.

Also you are conflating coding standard with ASCII layout. Here is a proper coding standard:

  1. Code review any method over 5 lines (average should be about 3).
  2. All features are accompanied by an acceptance test (black box).
  3. All classes have associated unit tests.
  4. Error messages are calculated in the top level controllers, just throw an exception of the appropriate type with just information in the message.
  5. All coding is done in feature branches. Please add current working branches to continuous integration and remove them when merged.

No ASCII OCD attacks. And really, is most of your time wasted chasing down misunderstood requirements because you had a bracket on the same line?

Sorry to be harsh, but you are doing yourself and your organisation a lot of damage with this tosh. If anyone who’s any good sees your “coding standard” in an interview they will likely look for work elsewhere. Swap the “coding standard” for pair programming and your whole world will be better.

“All of the above are completely new rules that are to be strictly enforced”. C’mon. Are you really going to stand up in an industrial tribunal or court of law and tell the judge that you sacked someone because they put their brackets on the next line?

yours, Marcus

Lastcraft, who are you replying to?

For myself I’d like to think my code is very legible. Unlike most programmers I do have coursework in graphic design since I hold a theater major (long story). I’ve laid out entire books and I pride myself in code legibility.

But I don’t have very many hard and fast rules, just guidelines. Formatting is about consistency, but it’s also about avoiding problems with the
the way the human brain reads things (like the fact this sentence has ‘the’ twice in a row just now, but because of the line break most of you won’t notice that until it’s pointed out to you).

With rules of any sort its important to know what they are, then learn what they seek to accomplish, and then you will know when they don’t apply and need to be ignored.

And formatting rules quite often need to be adapted rather than followed blindly.

Something also has to be said for avoiding situations where the conventions need to be broken down. Functions with enough arguments to warrant putting them on one line at a time to make them legible is a great example. Multiline if statements is another. It’s not always possible to avoid the problem, but it should prompt the question of “is there a better way here?”

EDIT: I’d personally do that one array like this


$month_length = array(
  'jan' => 31, 'feb' => 28, 'mar' => 31, 
  'apr' => 30, 'may' => 31, 'jun' => 30, 
  'jul' => 31, 'aug' => 31, 'sep' => 30, 
  'oct' => 31, 'nov' => 30, 'dec' => 31
); 

Less horizontal whitspace, and the end of the statement is more clear since it’s in tab line with the start.

Can I say it again? personal preference. :stuck_out_tongue:

Trying to argue it logically makes little sense in most cases.

Art being what it is, personal preferences are important but teachers in the arts do require consistency in approach. The same applies to code. If the team is using closed brace style, everyone should do it for consistency.

This actually applies to all walks of life. I’m getting ready to approach the pastor of my parish for permission to wear an alb when I am the cantor because while our parish tradition has been for the cantor to be in a Sunday suit, it is inconsistent with everyone else past the altar rail wearing albs. I’d prefer the to break the tradition to be more consistent with everyone else’s appearance during the mass.

(Besides, the much older tradition is for the cantor to be in an alb and perhaps a choir stole).

True but as I said, most decent IDEs have the option of source code formatting making even consistency irrelevant for the most part. Lets have a debate about proper syntax highlighting colours too :wink: Although, as we see in the OP there is potential for things which are entirely redundant and that does need consideration.

After that, I have no idea what you said…

This is my coding standard, I encourage everyone to follow this w3P standard:


$month_length = array(
'jan' => 31, 

'feb' => 28,


'mar' => 31, 



'apr' => 30, 




'may' => 31, 





'jun' => 30, 







'jul' => 31
); 

The more array pairs I have, the more spacing I increase.

I wish that PHP had a unified coding standard, something like Python’s PEP-8. Most PHP code is terribly written, despite what all PHP coders seem to claim, because so much variation is permitted in PHP. The standard itself is not as important, but something to increase the consistency in PHP code across the board would be beneficial.

Flexibility is good, but I think that PHP missed the balance between flexibility and legibility.

:rolleyes: Please tell me you’re trolling.

That is so cool! :smiley:

You got me thinking, I’ll incorporate some methods to make my code more pleasant to look at:


$ILikeMyFairy = true;

/* Fairy of my dreams

          .--.   _,
      .--;    \\ /(_
     /    '.   |   '-._    . ' .
    |       \\  \\    ,-.)  -= * =-
     \\ /\\_   '. \\((` .(    '/. '
      )\\ /     \\ )\\  _/   _/
     /  \\\\    .-'   '--. /_\\
    |    \\\\_.' ,        \\/||
    \\     \\_.-';,_) _)'\\ \\||
     '.       /`\\   (   '._/
       `\\   .;  |  . '.
    jgs  ).'  )/|      \\
         `    ` |  \\|   |
                 \\  |   |
                  '.|   |
                     \\  '\\__
                      `-._  '. _
                         \\`;-.` `._
                          \\ \\ `'-._\\
                           \\ |
                            \\ )
                             \\_\\



*/

assert($ILikeMyFairy == true);

// continue with application...
// ...

There! That’s what I’m talking about! This is one great fairy standard I’m gonna adopt for my coding. :stuck_out_tongue:

Lol, you bet I am. :smiley: Well my point is that, this topic has been discussed ever since I started programming 10 years ago and everyone just simply have their own kind of standards one way or another. Its impossible for one to agree upon another when PHP is so open. Things like double and single quotes is fine because it impacts the performance but spacings? Spare me on that, just don’t be as ridiculous as my “w3p standard”.

ethanp , fact that your code is an unreadable mess has more to do with poor skills, then the lack of strict coding standards in php. Go back to your python … or even better - java. YOu will be happy there.

mefisto, I think you missed the point. Each PHP coder’s code usually makes sense in isolation (i.e. in projects containing only their own code). The problem arises when you bring in third-party code. For example, the coding standards for PEAR, CakePHP, and Zend Framework all differ. Therefore, using these tools in any combination will result in fairly unaesthetic, inconsistent code.

A universal PHP coding style guide might be a good step towards solving this problem. Everyone, particularly library authors, could use these guidelines to write their code, instead of devising their own standard.

you mean like this :
http://framework.zend.com/manual/en/coding-standard.coding-style.html
and this
https://trac.cakephp.org/wiki/Developement/CodingStandards

But does this matter? Get an IDE with a code formatter and the problem is resolved without forcing people to use a standard which they may feel limits their productivity.