Clean Code

This workshop is an introduction to the principles outlined by Robert C. Martin; on how to write clean code. We'll refactor code form popular open-source projects, and learn core concepts along the way.

Transcript

1. Clean Code
3. SOLID
4. Single responsibility principle A class should only have one reason to change
5. Single responsibility principle interface Basket { public function getItems(); public function addItem($item); public function removeItem($item); public function calculateTotal(); public function processPayment($account, $amount); }
6. Single responsibility principle interface BasketCollection { public function getItems(); public function addItem($item); public function removeItem($item); } interface BasketPaymentProcessor { public function calculateTotal(BasketCollection $collection); public function processPayment($account, $amount); }
7. Open-closed principle A class should be easy to extend without changes
8. Open-closed principle class BasketPaymentProcessor { public function processPayment($account, $amount, $adapter) { switch ($adapter) { case "paypal": $this->chargeWithPaypal($account, $amount); break; case "payfast": $this->chargeWithPayfast($account, $amount); break; } } }
9. Open-closed principle interface Adapter { public function charge($account, $amount); } class BasketPaymentProcessor { protected $adapters = []; public function addAdapter($key, Adapter $adapter) { $this->adapters[$key] = $adapter; }
10. Open-closed principle public function processPayment($account, $amount, $adapter) { $this->adapters[$adapter]->charge($account, $amount); } } class DebitAdapter implements Adapter { public function charge($account, $amount) { // ...process some payment } }
11. Open-closed principle $processor = new BasketPaymentProcessor(); $processor->addAdapter( "paypal", new PaypalAdapter() ); $processor->addAdapter( "payfast", new PayfastAdapter() ); $processor->addAdapter( "debit", new DebitAdapter() );
12. Lizkov substitution principle Subclasses mustn't change expected behaviour
13. Lizkov substitution principle class Mailer { public function send($from, $to, $subject, $body) { mail($to, $subject, $body, "From: {$from}\n"); } }
14. Lizkov substitution principle class SecureMailer extends Mailer { public function send($from, $to, $subject, $body) { assert(filter_var($from, FILTER_VALIDATE_EMAIL)); assert(filter_var($to, FILTER_VALIDATE_EMAIL)); parent::send($from, $to, $subject, $body); } }
15. Lizkov substitution principle $mailer = new Mailer(); $mailer->send( "Chris <cgpitt@gmail.com>", "user@example.com", "...", "..." ); // works!
16. Lizkov substitution principle $mailer = new SecureMailer(); $mailer->send( "Chris <cgpitt@gmail.com>", "user@example.com", "...", "..." ); // breaks
17. Lizkov substitution principle function sendMail(Mailer $mailer) { print "send some mail..."; } //$mailer = new Mailer(); $mailer = new SecureMailer(); sendMail($mailer);
18. Interface segregation principle Interfaces shouldn't force classes to implement useless methods
19. Interface segregation principle interface CacheInterface { public function connect(array $credentials); public function set($key, $value); public function get($key); }
20. Interface segregation principle class FilesystemCache implements CacheInterface { public function connect(array $credentials) { // ...this does nothing! } }
21. Interface segregation principle interface ConnectionInterface { public function connect(array $credentials); } interface CacheInterface { public function set($key, $value); public function get($key); }
22. Interface segregation principle class DatabaseCache implements CacheInterface, ConnectionInterface { } class FilesystemCache implements CacheInterface { }
23. Interface segregation principle class CacheManager { public function connect() { if ($this->adapter instanceof ConnectionInterface) { $this->adapter->connect([...]); } } }
24. Dependency inversion principle High-level and low-level code should depend on abstractions
25. Dependency inversion principle class Logger { public function write($message) { file_put_contents(__DIR__ . "/application.log", $message); } }
26. Dependency inversion principle class Logger { protected $writer; public function __construct(Writer $writer) { $this->writer = $writer; } public function write($message) { $this->writer->write($message); } }
27. Test-driven development
28. Write a test → class CalculatorTest extends PHPUnit_Framework_TestCase { public function testItAdds() { $calculator = new Calculator(); $this->assertEquals( 4, $calculator->add(2, 2) ); } }
29. Write a fix → class Calculator { public function add($one, $two) { return 4; } }
30. Add another test → $calculator = new Calculator(); $this->assertEquals( 4, $calculator->add(2, 2) ); $this->assertEquals( 3, $calculator->add(1, 2) );
31. Write another fix → class Calculator { public function add($one, $two) { return $one + $two; } }
32. Meaningful Names
33. Use intention-revealing names Names should describe what things are are, how they are measured, what they do
34. Use intention-revealing names $d = 0; // elapsed time in days $elapsedTimeInDays = 0; function calculate($amount, $percentage) { return ($amount * $percentage) + $amount; } function calculateIncreaseWithPercentage($amount, $percentage) { return ($amount * $percentage) + $amount; }
35. Avoid disinformation Don't call something it's not!
36. Avoid disinformation $numberOfUsers = null; function addComment($text) { if (empty($text)) { return; } storeCommentInDatabase($text); }
37. Make meaningful distinctions Variables represent different things, and should be named to show that
38. Make meaningful distinctions $list1 = ["chris"]; $list2 = []; foreach ($list1 as $name) { $list2[] = ucwords($name); } function merge($array1, $array2) { foreach ($array1 as $key => $value) { $array2[$key] = $array1[$key]; } }
39. Avoid encodings Don't attach volatile or redundant meaning to names
40. Avoid encodings $strName = "Chris"; $dblPrice = 10; interface RepositoryInterface { }
41. Class names should be nouns Classes lead to objects which are things, not actions
42. Method names should start with a verb Methods are actions performed on and with things, they are actions
43. Don't be cute If names need your sense of humour to understand, they are not good names
44. Don't be cute public function kapow() { $this->repository->truncate(); } public function truncateRepository() { $this->repository->truncate(); }
45. Pick one word per concept Using synonyms for the same action, on different objects, is confusing
46. Pick one word per concept public function get($key, $default = null) { // ...gets a value with key, or returns default } public function retrieve($key, $default = null) { // ...gets a value with key, or returns default } public function read($key, $default = null) { // ...gets a value with key, or returns default }
47. Add meaningful context If the good name you've picked is still confusing, out of context, add some context
48. Add meaningful context class Product { public function purchaseWithAccount($account) { // ...enough context } } function purchaseProductWithAccount(Product $product, $account) { }
49. Functions
50. Make functions short Many shorter functions are better than fewer longer ones
51. Make functions do one thing If the name includes "and" or doesn't describe all the functionality well; it's doing too much
52. Don't use switch Using switch invites modification and adds responsibility
53. Be consistent with arguments If you have many "add" methods; make them have the same argument names and order
54. Don't use flag arguments Because calling functions with "true" and "false" as arguments says nothing about the arguments
55. Use objects/arrays when you need many arguments A single type is easier to understand than many arguments which form that type
56. Avoid side-effects Side-effects are hidden functionality, extra responsibility, stateful
57. Separate commands and queries Functions should either do something or answer something; doing both is confusing
58. Don't repeat yourself Functions are for reuse, not encapsulating repeated code
59. Don't mix concepts Don't abstract for things that can change independently
60. Comments
61. Comments represent bad code If you need comments to understand what the code is doing then the code is poorly written
62. Legal comments are ok Comments which show things like licenses, copyright and authors
63. Legal comments are ok /** * Licensed under the ACME PSL License. * * This source file is subject to the ACME PSL License that is * bundled with this package in the license.txt file. * * @package Acme\Explosions * @author ACME LLC * @license ACME PSL * @copyright (c) 2011-2014, ACME LLC * @link http://acme.com */
64. Warning comments are ok Could the code cause problems if it is used? Should the reader refer to other code?
65. Warning comments are ok /** * @group LongRunning */ public function testFullRequestCycle() { // Don't run this if you're short on time! } public function seedDatabase() { // This will overwrite production data... }
66. Todo comments are ok Sometimes you can't make the changes you expect, and an IDE will point these out later
67. Todo comments are ok public function getFormattedProduct(Product $product) { // TODO check for errors in product data return [ "title" => $this->getFormattedTitle($product->title), "price" => $this->getFormattedPrice($product->price) ]; }
68. Type comments are ok In a dynamic language, documenting types is helpful, and an IDE will show inconsistencies
69. Type comments are ok /** * @param Responder $responder * @param array $input * * @return Response */ public function validateInput(Responder $responder, array $input) { if ($this->validator->passes($input)) { return $responder->respondWithOk(); } return $responder->respondWithErrors($this->validator); }
70. Formatting
71. Add spaces between concepts Properties and methods are easier to read when there are spaces between them
72. Add spaces between concepts public function processPayment($account, $amount) { $chargeId = $this->adapter->chargeWithAmount($amount); if ($this->adapter->isSuccessful($chargeId)) { $this->repository->updateAccount($account); } }
73. Order methods by depth If method "a" depends on "b" and then "c", that is the order in which they should appear
74. Order methods by depth public function getSummaryArray() { return [ "title" => $this->getTitle(), ... ]; } protected function getTitle() { return ucwords($this->title); }
75. Order methods by conceptual affinity If methods "d" and "e" do the similar things, put them close together
76. Order methods by conceptual affinity class TestCase { public function assertProductValid(Product $product) { // ...check the product } public function assertChargeValid(Charge $charge) { // ...check the charge } }
77. Limit the line length Readers will already have to scroll vertically, don't make that horizontally also.
78. Have team standards Being consistent is important, even if your standards are strange
79. Have team standards → php-fig.org/psr/psr-1 → php-fig.org/psr/psr-2 → symfony.com/doc/current/contributing/code/standards.html → laravel.com/docs/4.2/contributions#coding-style → framework.zend.com/wiki/display/ZFDEV2/Coding+Standards