-
Comment block, have 1 break line in every tags.
Hmmm
/** * Description * @param string $a * @param int $b * @return \OmiseObject */
Better
/** * Description * * @param string $a * @param int $b * * @return \OmiseObject */
Why?
This approach aimed to increase a readability of a code.
Think about we have bunch of tags in a DocBlock like below/** * This method will send a connection to A. * Then, observing it and dispatch to B if success. * @param \Dispatcher $dispatcher * @param string $code * @param int $amount * @param array $params * @return void * @throws \DispatchException * @see http://www.google.com */ public function observer($dispatcher, $code, $amount, $params) { // ... }
The below approach makes code much more easier to read.
/** * This method will send a connection to A. * Then, observing it and dispatch to B if success. * * @param \Dispatcher $dispatcher * @param string $code * @param int $amount * @param array $params * * @return void * * @throws \DispatchException * * @see http://www.google.com */ public function observer($dispatcher, $code, $amount, $params) { // ... }
-
For any methods in a class, must provide at least a
@param
document if that method is defined an argument unless you usetype-hint
Hmmm
class A { public function haveFood($name) { // ... } public function pay($amount) { // ... } }
Better
class A { /** * @param string $name */ public function haveFood($name) { // ... } /** * @param int $amount */ public function pay($amount) { // ... } }
Why?
To make other developers easily understand your code since an argument itself cannot tell a type, it could be any ofstring
,int
or evenobject
.
Note, unless you usetype-hint
approach./** * This method will send a connection to A. * Then, observing it and dispatch to B if success. * * @return void * * @throws \DispatchException * * @see http://www.google.com */ public function observer( \Dispatcher $dispatcher, string $code, int $amount, array $params ) { // ... }
-
1 space between
!condition
.Hmmm
if (!$paid) { // ... }
Better
if (! $paid) { // ... }
Why?
To increase readability of a code.Inspired by http://github.com/laravel/framework project.
-
No shorthand syntax for a conditional block.
Hmmm
if (! $paid) echo "failed";
Better
if (! $paid) { echo "failed"; }
Why?
Some of developers implemented with the first style and some of them work with the second style.
This approach just to make a standard by picking only 1 style.So, if you want to go with the first style, just go with it for the rest of the code. Or if you want to go with the second style, just also go with it for your whole project.
(so I just picked second, to avoid some unintensionally mistake from brackets).
- Always has 1 breakline at the end of a file.
-
Formatting code vertically alignment (nice to have)
Hmmm
$person->FirstName = "Chris"; $person->Surname = "McGrath"; $person->Age = 24; $person->Occupation = "Software Developer"; $person->HomeTown = "Brisbane";
Better
$person->FirstName = "Chris"; $person->Surname = "McGrath"; $person->Age = 24; $person->Occupation = "Software Developer"; $person->HomeTown = "Brisbane";
note, example from http://www.codealignment.com/Details.html
Why?
To increase readability of a code.
-
Continue from 5th. If 2 part of code contexts seems different, they don't need to do vertically alignment to each other.
Hmmm
// Init personal information $person->FirstName = "Chris"; $person->Surname = "McGrath"; $person->Age = 24; $person->Occupation = "Software Developer"; $person->HomeTown = "Brisbane"; // Perform insert record. if ($person->save()) { $log = new Logger; $log->name = 'my-log.txt'; $log->create(); }
Better
// Init personal information $person->FirstName = "Chris"; $person->Surname = "McGrath"; $person->Age = 24; $person->Occupation = "Software Developer"; $person->HomeTown = "Brisbane"; // Perform insert record. if ($person->save()) { $log = new Logger; $log->name = 'my-log.txt'; $log->create(); }
Why?
To increase readability of a code.
-
1 space for variable string concatenation
Hmmm
$name = 'Guzzilar'; $sentence = 'Have a nice day!'; echo "Hello, ".$name.". ".$sentence; // Hello, Guzzilar. Have a nice day!
Better
$name = 'Guzzilar'; $sentence = 'Have a nice day!'; echo "Hello, " . $name . ". " . $sentence; // Hello, Guzzilar. Have a nice day!
Why?
To increase readability of a code.
-
Avoid
getter
andsetter
if those methods do nothing more than serving private property.Hmmm
class Acme { /** * @var string */ protected $name = 'Guzzilar'; /** * @var int */ protected $age = 99; /** * @var string */ protected $occupation = 'Developer'; public function getName() { return $this->name; } public function getAge() { return $this->age; } public function getOccupation() { return $this->occupation; } /** * @param string $name */ public function setName($name) { return $this->name = $name; } /** * @param int $age */ public function setAge($age) { return $this->age = $age; } /** * @param string $occupation */ public function setOccupation($occupation) { return $this->occupation = $occupation; } }
Better
class Acme { /** * @var string */ protected $name = 'Guzzilar'; /** * @var int */ protected $age = 99; /** * @var string */ protected $occupation = 'Developer'; public function getName() { return $this->name; } public function getAge() { return $this->age; } public function getOccupation() { return $this->occupation; } /** * @param array $data */ public function updatePersonalInformation($data) { $this->name = $data['name']; $this->age = $data['age']; $this->occupation = $data['occupation']; } }
Even Better
// Rename class name to more expose an information. class Person { /** * @var string */ protected $name = 'Guzzilar'; /** * @var int */ protected $age = 99; /** * @var string */ protected $occupation = 'Developer'; /** * Ask yourself why do we need 'get' information here? * What is a use case that requires this method? * Is it possible to implement it with 'Tell, Don't Ask' principle or something else? */ public function getName() { return $this->name; } public function getAge() { return $this->age; } public function getOccupation() { return $this->occupation; } /** * @param array $data */ public function updateInformation($data) { $this->name = $data['name']; $this->age = $data['age']; $this->occupation = $data['occupation']; } }
-
Use
@test
tag for test method.Hmmm
public function testCatShouldEatByUsingMouth() { // ... }
Better
/** * @test */ public function cat_should_eat_by_using_mouth() { // ... }
Why?
To increase readability of a code.
-
Just use normal-human english sentence style for a test case
Hmmm
/** * @test */ public function saySorryIfTicketWasSoldOut() { // ... }
Better
/** * @test */ public function say_sorry_if_ticket_was_sold_out() { // ... }
- Consider to use SOLID principle. Especially, a Single Responsibility principle.
- Consider to use Tell, Don't Ask principle
-
Avoid using
boolean
as one of a parameter.
added on Mar 2, 2017Hmmm
/** * @param string $id * @param bool $include_header * @param bool $include_footer */ public function exportData($id, $include_header = false, $include_footer = false) { // ... }
Better
/** * @param string $id * @param array $config */ public function exportData($id, $config = array()) { // ... }
Even Better
class Exporter { public function __construct(\Template $template) { // ... } /** * @param string $id */ public function exportData($id) { $this->template->generate(...); } // ... }
Why?
Boolean is not clear enough if you consider from a caller side.
As from the example above, let's execute it.$exporter = new Exporter; $exporter->exportData('report_id', true, false);
Now let's assume that you never see the example code before, can you guess an output from the 2nd and 3rd parameters?
How about this one,
$exporter = new Exporter; $exporter->exportData('report_id', array('includeHeader' => true, 'includeFooter' => false)
Or even you can go with the 'Even Better' approach.
$exporter = new Exporter(Template::include('header', 'footer')); $exporter->exportData('report_id');
That we can take a responsibility to determine using template out from the Exporter class. Which is we can apply a 'Single Responsibility' principle here.
And since you do dependency injection by passing a Template class. Now you can easily mock a Template class when write a test, or even turn that class to an interface class that you can take-in/out any template you want without any effect to the gathering data part.suggested solution by @robinclart as from a discussion at issue#1