mainSavannah Administration - Support: sr #109711, PHP code style of savane

 
 

sr #109711: PHP code style of savane

Submitter:  Peter Liscovius <peterdd>
Submitted:  Wed 19 Jun 2019 07:02:37 PM UTC
   
 
Category:  Savannah website Priority:  5 - Normal
Severity:  3 - Normal Status:  Confirmed
Privacy:  Public Assigned to:  ineiev
Operating System:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 20 Jun 2019 06:05:23 AM UTC, comment #1: 

Partially agreed. I'm more used to the coding style from the GNU coding standards.

Ineiev <ineiev>
Site Administrator
Wed 19 Jun 2019 07:02:37 PM UTC, original submission:  

Coding styles are different between programming languages, different between companies, also different between communities of a programming language by countries sometimes, and even change over time.

Netherless I want to point out some rules that SHOULD be applied to the PHP code of savane when changing a file or writing a new .php file:

  • Do not put any spaces between a function call and the opening parenthesis.

  It makes reading and searching sourcetrees easier (a grep -rin 'time(' would not match 'Time to say goodbye.', but any variant of time( Time( tIME( ...

This was the vote result for PSR-2:


function_space_after:
    no: 22


Clearly.

  • Always put the body following an if structure within curly brackets, even if it just contains a single expression!

  Not putting curly brackets there is like laying hidden mines and provokes hard to find bugs for any later person.
  I don't know why people still keep doing that evil shortcut. Maybe it is from C-coders that used it 40 years ago to optimize their compiler that couldn't optimize {} out when a single command follows? I don't know.

There is https://www.php.net/manual/en/control-structures.if.php , but that guys are C-coders too.
  For single expression if's there is the ternary conditional operator (but I find it easier write and read read 'if ($x){ expression } else { expression }' constructs)
  'if ($x) ? expression : expression;'

This was the vote result for PSR-2:

+verbstim+
always_use_control_braces:
    no: 3
    yes: 19
-verbatim-

Quite clearly.

  • Please optionally put . in PHP comments only if it is a full sentence. I even prefer not to put a . even for a single sentense in comment, but that is not important.


  • Do not indent curly brackets at nowhere spaces. Either align it

  * vertical with the if/while/switch/for/.. 
  * opening curly bracket at same line, ending bracket vertical aligned with if/while/switch/for(...

The other whole bunch of coding styles are debatable, but these 4 are IMHO important to follow.

General:

  • Only change code that matches the commit message. Stick to an existing coding style of a file. Keeps the changes to git commits small and makes reviews/git history/git blame/ easier.


https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-1-basic-coding-standard.md

There is also PSR-2 https://www.php-fig.org/psr/psr-2/, but I would not fully agree to that standard (See the quite diverting voting results at https://www.php-fig.org/psr/psr-2/#a1-survey-data
https://www.php-fig.org/psr/psr-2/

I disagree to that PSR-2 rule:

-verbatim+
indent_type:
    tab: 7 (I prefer this as a user of also dumb/basic editors, navigation easier)
    2 spaces: 1
    4 spaces: 14
-verbatim-

The other rules of PSR-2 are either ok or I do not care of the flavor and can adapt to it.

Peter Liscovius <peterdd>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ineiev (Posted a comment)
  • -email is unavailable- added by peterdd (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-06-20 ineiev StatusNone Confirmed
        Assigned toNone ineiev
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code