Your code sucks, let's fix it.

How do you measure the quality of your code? Performance and testing are just one aspect of code, in order to meet deadlines and make maintenance quicker you also need your code to be readable, decoupled and generally easier to comprehend and work with. This talk will go over tips and exercises to help you identify trouble areas, refactor them and train you to write better code in future projects. Come make your code look and function better.

Transcript

1. Your code sucks, let’s !x it! Object Calisthenics and Code readability Rafael Dohms Tuesday, February 28, 12
2. Rafael Dohms PHP Evangelist, Speaker and contributor. He is a very active member of the PHP Community, having helped create and manage two PHP User Groups in Brazil. He shared the lead of PHPSP for 3 wonderful years making a positive mark on the local market. Developer, gamer and lover of code he also hosts Brazil’s first PHP Podcast: PHPSPCast, as well as contributing to well known projects. He moved to the Netherlands in search of new challenges and is now part of the team at WEBclusive. photo credit: Eli White Tuesday, February 28, 12
3. What’s the talk about? •Why does my code suck? •How can we fix it? Tuesday, February 28, 12
4. Why does my code suck? Tuesday, February 28, 12
5. Why does my code suck? Is it Readable? Tuesday, February 28, 12
6. Why does my code suck? Is it Readable? Is it Testable? Tuesday, February 28, 12
7. Why does my code suck? Is it Readable? Is it Testable? Is it Maintainable? Tuesday, February 28, 12
8. Why does my code suck? Is it Readable? Is it Testable? Is it Maintainable? Is it Reusable? Tuesday, February 28, 12
9. Does it look like this? <?php $list=mysql_connect("******","*******","*****"); if(!$list)echo 'Cannot login.'; else{ mysql_select_db("******", $list); $best=array("0","0","0","0","0","0"); $id=mysql_num_rows(mysql_query("SELECT * FROM allnews")); $count=0; for($i=0;$i<6;$i++){ while(mysql_query("SELECT language FROM allnews WHERE id=$id-$count")!="he")$count+ +; $best[$i]=mysql_query("SELECT id FROM allnews WHERE id=$id-$count");} $id=$id-$count; $maxdate=mktime(0,0,0,date('m'),date('d')-7,date('Y')); while(mysql_query("SELECT date FROM allnews WHERE id=$id-$count")>=$maxdate){ if(mysql_query("SELECT language FROM allnews WHERE id=$id-$count")=="he"){ $small=$best[0]; while($i=0;$i<6;$i++){ if(mysql_query("SELECT score FROM allnews WHERE id= $small)"<mysql_query("SELECT score FROM allnews WHERE id=$best[i+1]")) $small=$best[i+1];} if(mysql_query("SELECT score FROM allnews WHERE id= $small")<mysql_query("SELECT score FROM allnews WHERE id=$id-$count")){ while($i=0;$i<6;$i++){ if($small==$best[i])$best[i]=mysql_query("SELECT id FROM allnews WHERE id=$id-$count");}}}} while($i=0;$i<6;$i++) echo '<a href="news-page.php?id='.$best[i].'"><div class="box '.mysql_query("SELECT type FROM allnews WHERE id=$best[i]").'">'.mysql_query("SELECT title FROM allnews WHERE id= $best[i]").'<div class="img" style="background-image:url(images/'.mysql_query("SELECT image1 FROM allnews WHERE id=$best[i]").');"></div></div></a>'; mysql_close($list); } ?> Tuesday, February 28, 12
10. Does it look like this? <?php $list=mysql_connect("******","*******","*****"); if(!$list)echo 'Cannot login.'; else{ mysql_select_db("******", $list); $best=array("0","0","0","0","0","0"); $id=mysql_num_rows(mysql_query("SELECT * FROM allnews")); $count=0; for($i=0;$i<6;$i++){ while(mysql_query("SELECT language FROM allnews WHERE id=$id-$count")!="he")$count+ +; $best[$i]=mysql_query("SELECT id FROM allnews WHERE id=$id-$count");} $id=$id-$count; $maxdate=mktime(0,0,0,date('m'),date('d')-7,date('Y')); while(mysql_query("SELECT date FROM allnews WHERE id=$id-$count")>=$maxdate){ if(mysql_query("SELECT language FROM allnews WHERE id=$id-$count")=="he"){ $small=$best[0]; while($i=0;$i<6;$i++){ if(mysql_query("SELECT score FROM allnews WHERE id= $small)"<mysql_query("SELECT score FROM allnews WHERE id=$best[i+1]")) $small=$best[i+1];} if(mysql_query("SELECT score FROM allnews WHERE id= $small")<mysql_query("SELECT score FROM allnews WHERE id=$id-$count")){ while($i=0;$i<6;$i++){ if($small==$best[i])$best[i]=mysql_query("SELECT id FROM allnews WHERE id=$id-$count");}}}} while($i=0;$i<6;$i++) echo '<a href="news-page.php?id='.$best[i].'"><div class="box '.mysql_query("SELECT type FROM allnews WHERE id=$best[i]").'">'.mysql_query("SELECT title FROM allnews WHERE id= $best[i]").'<div class="img" style="background-image:url(images/'.mysql_query("SELECT image1 FROM allnews WHERE id=$best[i]").');"></div></div></a>'; mysql_close($list); } ?> If Rebecca Black was a developer Tuesday, February 28, 12
11. How do we fix it? Tuesday, February 28, 12
12. Object Calisthenics Tuesday, February 28, 12
13. Object Calisthenics cal·is·then·ics - noun - /ˌkaləsˈTHeniks/ Calisthenics are a form of dynamic exercise consisting of a variety of simple, often rhythmical, movements, generally using minimal equipment or apparatus. Tuesday, February 28, 12
14. Object Calisthenics cal·is·then·ics - noun - /ˌkaləsˈTHeniks/ Calisthenics are a form of dynamic exercise consisting of a variety of simple, often rhythmical, movements, generally using minimal equipment or apparatus. A variety of simple, often rhythmical, exercises to achieve better OO and code quality Tuesday, February 28, 12
15. Object Calisthenics “So here’s an exercise that can help you to internalize principles of good object-oriented design and actually use them in real life.” -- Jeff Bay Tuesday, February 28, 12
16. Object Calisthenics “So here’s an exercise that can help you to internalize principles of good object-oriented design and actually use them in real life.” -- Jeff Bay Important: PHP != JAVA Adaptations will be done Tuesday, February 28, 12
17. Object Calisthenics + Readability Tips Tuesday, February 28, 12
18. Object Calisthenics + Readability Tips “You need to write code that minimizes the time it would take someone else to understand it—even if that someone else is you.” -- Dustin Boswell and Trevor Foucher Tuesday, February 28, 12
19. Object Calisthenics + Readability Tips “You need to write code that minimizes the time it would take someone else to understand it—even if that someone else is you.” -- Dustin Boswell and Trevor Foucher I’m a tip Tuesday, February 28, 12
20. Disclaimer: “These are guidelines, not rules” Tuesday, February 28, 12
21. OC #1 “Only one indentation level per method” Tuesday, February 28, 12
22. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } } return $valid; } Tuesday, February 28, 12
23. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } } return $valid; } 0 1 2 3 Tuesday, February 28, 12
24. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } } return $valid; } 0 1 2 3 Tuesday, February 28, 12
25. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $validationResult = validateSingleProduct($rawProduct, $requiredFields); if ( ! $validationResult){ $valid = false; } } return $valid; } function validateSingleProduct($product, $requiredFields) { $valid = true; $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } return $valid; } Tuesday, February 28, 12
26. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $validationResult = validateSingleProduct($rawProduct, $requiredFields); if ( ! $validationResult){ $valid = false; } } return $valid; } function validateSingleProduct($product, $requiredFields) { $valid = true; $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } return $valid; } whitespace Tuesday, February 28, 12
27. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $validationResult = validateSingleProduct($rawProduct, $requiredFields); if ( ! $validationResult){ $valid = false; } } return $valid; } function validateSingleProduct($product, $requiredFields) { $valid = true; $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } return $valid; } whitespace duplicated logic Tuesday, February 28, 12
28. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $validationResult = validateSingleProduct($rawProduct, $requiredFields); if ( ! $validationResult){ $valid = false; } } return $valid; } function validateSingleProduct($product, $requiredFields) { $valid = true; $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } return $valid; } whitespace duplicated logic 0 1 2 0 1 2 Tuesday, February 28, 12
29. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $validationResult = validateSingleProduct($rawProduct, $requiredFields); if ( ! $validationResult){ $valid = false; } } return $valid; } function validateSingleProduct($product, $requiredFields) { $valid = true; $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } return $valid; } 0 1 2 0 1 2 Tuesday, February 28, 12
30. function validateProducts($products) { // Check to make sure that our valid fields are in there $requiredFields = array( 'price', 'name', 'description', 'type', ); $valid = true; foreach ($products as $rawProduct) { $validationResult = validateSingleProduct($rawProduct, $requiredFields); if ( ! $validationResult){ $valid = false; } } return $valid; } function validateSingleProduct($product, $requiredFields) { $valid = true; $fields = array_keys($rawProduct); foreach ($requiredFields as $requiredField) { if (!in_array($requiredField, $fields)) { $valid = false; } } return $valid; } 0 1 2 0 1 2 Tuesday, February 28, 12
31. function validateProducts($storeData) { $requiredFields = array('price','name','description','type'); foreach ($storeData['products'] as $rawProduct) { if ( ! validateSingleProduct($rawProduct, $requiredFields)) return false; } return true; } function validateSingleProduct($product, $requiredFields) { $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) == 0); } Tuesday, February 28, 12
32. function validateProducts($storeData) { $requiredFields = array('price','name','description','type'); foreach ($storeData['products'] as $rawProduct) { if ( ! validateSingleProduct($rawProduct, $requiredFields)) return false; } return true; } function validateSingleProduct($product, $requiredFields) { $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) == 0); } I see cheating! Tuesday, February 28, 12
33. function validateProducts($storeData) { $requiredFields = array('price','name','description','type'); foreach ($storeData['products'] as $rawProduct) { if ( ! validateSingleProduct($rawProduct, $requiredFields)) return false; } return true; } function validateSingleProduct($product, $requiredFields) { $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) == 0); } I see cheating! Single line IF, simple operations Tuesday, February 28, 12
34. function validateProducts($storeData) { $requiredFields = array('price','name','description','type'); foreach ($storeData['products'] as $rawProduct) { if ( ! validateSingleProduct($rawProduct, $requiredFields)) return false; } return true; } function validateSingleProduct($product, $requiredFields) { $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) == 0); } return early Single line IF, simple operations Tuesday, February 28, 12
35. function validateProducts($storeData) { $requiredFields = array('price','name','description','type'); foreach ($storeData['products'] as $rawProduct) { if ( ! validateSingleProduct($rawProduct, $requiredFields)) return false; } return true; } function validateSingleProduct($product, $requiredFields) { $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) == 0); } return early Single line IF, simple operations C (native) functions are faster then PHP Tuesday, February 28, 12
36. function validateProducts($storeData) { $requiredFields = array('price','name','description','type'); foreach ($storeData['products'] as $rawProduct) { if ( ! validateSingleProduct($rawProduct, $requiredFields)) return false; } return true; } function validateSingleProduct($product, $requiredFields) { $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) == 0); } Tuesday, February 28, 12
37. function validateProducts($storeData) { $requiredFields = array('price','name','description','type'); foreach ($storeData['products'] as $rawProduct) { if ( ! validateSingleProduct($rawProduct, $requiredFields)) return false; } return true; } function validateSingleProduct($product, $requiredFields) { $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) == 0); } Tuesday, February 28, 12
38. function validateProductList($products) { $invalidProducts = array_filter($products, 'isInvalidProduct'); return (count($invalidProducts) === 0); } function isInvalidProduct($rawProduct) { $requiredFields = array('price', 'name', 'description', 'type'); $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) > 0); } Tuesday, February 28, 12
39. function validateProductList($products) { $invalidProducts = array_filter($products, 'isInvalidProduct'); return (count($invalidProducts) === 0); } function isInvalidProduct($rawProduct) { $requiredFields = array('price', 'name', 'description', 'type'); $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) > 0); } faster iteration Tuesday, February 28, 12
40. function validateProductList($products) { $invalidProducts = array_filter($products, 'isInvalidProduct'); return (count($invalidProducts) === 0); } function isInvalidProduct($rawProduct) { $requiredFields = array('price', 'name', 'description', 'type'); $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) > 0); } faster iteration reusable method Tuesday, February 28, 12
41. function validateProductList($products) { $invalidProducts = array_filter($products, 'isInvalidProduct'); return (count($invalidProducts) === 0); } function isInvalidProduct($rawProduct) { $requiredFields = array('price', 'name', 'description', 'type'); $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) > 0); } faster iteration reusable method method name matches “true” result Tuesday, February 28, 12
42. function validateProductList($products) { $invalidProducts = array_filter($products, 'isInvalidProduct'); return (count($invalidProducts) === 0); } function isInvalidProduct($rawProduct) { $requiredFields = array('price', 'name', 'description', 'type'); $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) > 0); } faster iteration reusable method method name matches “true” result readable return: zero invalid products Tuesday, February 28, 12
43. function validateProductList($products) { $invalidProducts = array_filter($products, 'isInvalidProduct'); return (count($invalidProducts) === 0); } function isInvalidProduct($rawProduct) { $requiredFields = array('price', 'name', 'description', 'type'); $fields = array_keys($rawProduct); $missingFields = array_diff($requiredFields, $fields); return (count($missingFields) > 0); } faster iteration reusable method method name matches “true” result readable return: zero invalid products List is more readable the plural Tuesday, February 28, 12
44. Key Benefits •Cohesiveness •Methods does only one thing •Increases re-use Tuesday, February 28, 12
45. OC #2 “Do not use the ‘else’ keyword” Tuesday, February 28, 12
46. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } Tuesday, February 28, 12
47. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } Tuesday, February 28, 12
48. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } Tuesday, February 28, 12
49. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } Tuesday, February 28, 12
50. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } Tuesday, February 28, 12
51. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } Tuesday, February 28, 12
52. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } Tuesday, February 28, 12
53. public function createPost($request) { $entity = new Post(); $form = new MyForm($entity); $form->bind($request); if ($form->isValid()){ $repository = $this->getRepository('MyBundle:Post'); if (!$repository->exists($entity) ) { $repository->save($entity); return $this->redirect('create_ok'); } else { $error = "Post Title already exists"; return array('form' => $form, 'error' => $error); } } else { $error = "Invalid fields"; return array('form' => $form, 'error' => $error); } } intermediate variable intermediate variable Tuesday, February 28, 12
54. public function createPost($request) { $entity = new Post(); $repository = $this->getRepository('MyBundle:Post'); $form = new MyForm($entity); $form->bind($request); if ( ! $form->isValid()){ return array('form' => $form, 'error' => 'Invalid fields'); } if ($repository->exists($entity)){ return array('form' => $form, 'error' => 'Duplicate post title'); } $repository->save($entity); return $this->redirect('create_ok'); } Tuesday, February 28, 12
55. public function createPost($request) { $entity = new Post(); $repository = $this->getRepository('MyBundle:Post'); $form = new MyForm($entity); $form->bind($request); if ( ! $form->isValid()){ return array('form' => $form, 'error' => 'Invalid fields'); } if ($repository->exists($entity)){ return array('form' => $form, 'error' => 'Duplicate post title'); } $repository->save($entity); return $this->redirect('create_ok'); } Tuesday, February 28, 12
56. public function createPost($request) { $entity = new Post(); $repository = $this->getRepository('MyBundle:Post'); $form = new MyForm($entity); $form->bind($request); if ( ! $form->isValid()){ return array('form' => $form, 'error' => 'Invalid fields'); } if ($repository->exists($entity)){ return array('form' => $form, 'error' => 'Duplicate post title'); } $repository->save($entity); return $this->redirect('create_ok'); } removed intermediates Tuesday, February 28, 12
57. public function createPost($request) { $entity = new Post(); $repository = $this->getRepository('MyBundle:Post'); $form = new MyForm($entity); $form->bind($request); if ( ! $form->isValid()){ return array('form' => $form, 'error' => 'Invalid fields'); } if ($repository->exists($entity)){ return array('form' => $form, 'error' => 'Duplicate post title'); } $repository->save($entity); return $this->redirect('create_ok'); } removed intermediates early return Tuesday, February 28, 12
58. public function createPost($request) { $entity = new Post(); $repository = $this->getRepository('MyBundle:Post'); $form = new MyForm($entity); $form->bind($request); if ( ! $form->isValid()){ return array('form' => $form, 'error' => 'Invalid fields'); } if ($repository->exists($entity)){ return array('form' => $form, 'error' => 'Duplicate post title'); } $repository->save($entity); return $this->redirect('create_ok'); } Separate code into blocks. Its like using Paragraphs. removed intermediates early return Tuesday, February 28, 12
59. Key Benefits •Helps avoid code duplication •Makes code clearer (single path) Tuesday, February 28, 12
60. OC #3 “Wrap all primitive types and string, if it has behaviour” Adapted Tuesday, February 28, 12
61. class UIComponent { //... public function repaint($animate = true){ //... } } //... $component->repaint(false); Tuesday, February 28, 12
62. class UIComponent { //... public function repaint($animate = true){ //... } } //... $component->repaint(false); Tuesday, February 28, 12
63. class UIComponent { //... public function repaint($animate = true){ //... } } //... $component->repaint(false); unclear operation Tuesday, February 28, 12
64. class UIComponent { //... public function repaint( Animate $animate ){ //... } } class Animate { public $animate; public function __construct( $animate = true ) { $this->animate = $animate;} } //... $component->repaint( new Animate(false) ); Tuesday, February 28, 12
65. class UIComponent { //... public function repaint( Animate $animate ){ //... } } class Animate { public $animate; public function __construct( $animate = true ) { $this->animate = $animate;} } //... $component->repaint( new Animate(false) ); This can now encapsulate all animation related operations Tuesday, February 28, 12
66. Key Benefits •Type Hinting •Encapsulation of operations Tuesday, February 28, 12
67. OC #4 “Only one -> per line, if not getter or fluent” Adapted Tuesday, February 28, 12
68. $this->base_url = $this->CI->config->site_url().'/'.$this->CI->uri->segment(1).$this- >CI->uri->slash_segment(2, 'both'); $this->base_uri = $this->CI->uri->segment(1).$this->CI->uri->slash_segment(2, 'leading'); Source: CodeIgniter Tuesday, February 28, 12
69. $this->base_url = $this->CI->config->site_url().'/'.$this->CI->uri->segment(1).$this- >CI->uri->slash_segment(2, 'both'); $this->base_uri = $this->CI->uri->segment(1).$this->CI->uri->slash_segment(2, 'leading'); properties are harder to mock Source: CodeIgniter Tuesday, February 28, 12
70. $this->base_url = $this->CI->config->site_url().'/'.$this->CI->uri->segment(1).$this- >CI->uri->slash_segment(2, 'both'); $this->base_uri = $this->CI->uri->segment(1).$this->CI->uri->slash_segment(2, 'leading'); properties are harder to mock no whitespace Source: CodeIgniter Tuesday, February 28, 12
71. - Underlying encapsulation problem - Hard to debug and test - Hard to read and understand $this->base_url = $this->CI->config->site_url().'/'.$this->CI->uri->segment(1).$this- >CI->uri->slash_segment(2, 'both'); $this->base_uri = $this->CI->uri->segment(1).$this->CI->uri->slash_segment(2, 'leading'); properties are harder to mock no whitespace Source: CodeIgniter Tuesday, February 28, 12
72. - Underlying encapsulation problem - Hard to debug and test - Hard to read and understand $this->base_url = $this->CI->config->site_url().'/'.$this->CI->uri->segment(1).$this- >CI->uri->slash_segment(2, 'both'); $this->base_uri = $this->CI->uri->segment(1).$this->CI->uri->slash_segment(2, 'leading'); properties are harder to mock no whitespace move everything to uri object $this->getCI()->getUriBuilder()->getBaseUri(‘leading’); Source: CodeIgniter Tuesday, February 28, 12
73. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
74. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); fluent interface Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
75. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); operator alignment fluent interface Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
76. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); operator alignment fluent interface $user = $this->get('security.context')->getToken()->getUser(); Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
77. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); operator alignment fluent interface $user = $this->get('security.context')->getToken()->getUser(); only getters (no operations) Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
78. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); operator alignment fluent interface $user = $this->get('security.context')->getToken()->getUser(); only getters (no operations) Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
79. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); operator alignment fluent interface $user = $this->get('security.context')->getToken()->getUser(); only getters (no operations) where did my autocomplete go? Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
80. $filterChain->addFilter(new Zend_Filter_Alpha()) ->addFilter(new Zend_Filter_StringToLower()); operator alignment fluent interface $user = $this->get('security.context')->getToken()->getUser(); only getters (no operations) return null? where did my autocomplete go? Source: Zend Framework App Source: Symfony 2 Docs. Tuesday, February 28, 12
81. Key Benefits •Readability •Easier Mocking •Easier to Debug •Demeter’s Law Tuesday, February 28, 12
82. OC #5 “Do not Abbreviate” Tuesday, February 28, 12
83. if($sx >= $sy) { if ($sx > $strSysMatImgW) { $ny = $strSysMatImgW * $sy / $sx; $nx = $strSysMatImgW; } if ($ny > $strSysMatImgH) { $nx = $strSysMatImgH * $sx / $sy; $ny = $strSysMatImgH; } } else { if ($sy > $strSysMatImgH) { $nx = $strSysMatImgH * $sx / $sy; $ny = $strSysMatImgH; } if($nx > $strSysMatImgW) { $ny = $strSysMatImgW * $sy / $sx; $nx = $strSysMatImgW; } } Tuesday, February 28, 12
84. if($sx >= $sy) { if ($sx > $strSysMatImgW) { $ny = $strSysMatImgW * $sy / $sx; $nx = $strSysMatImgW; } if ($ny > $strSysMatImgH) { $nx = $strSysMatImgH * $sx / $sy; $ny = $strSysMatImgH; } } else { if ($sy > $strSysMatImgH) { $nx = $strSysMatImgH * $sx / $sy; $ny = $strSysMatImgH; } if($nx > $strSysMatImgW) { $ny = $strSysMatImgW * $sy / $sx; $nx = $strSysMatImgW; } } ? ? ? ? ? Tuesday, February 28, 12
85. Why? Tuesday, February 28, 12
86. Why? Its repeated many times, and i’m lazy. Tuesday, February 28, 12
87. Why? Its repeated many times, and i’m lazy. Underlying Problem! You need to transfer those operations into a separate class. Tuesday, February 28, 12
88. Why? function processResponseHeadersAndDefineOutput($response) { ... } Tuesday, February 28, 12
89. Why? This method name is too long to type, and i’m lazy. function processResponseHeadersAndDefineOutput($response) { ... } Tuesday, February 28, 12
90. Why? This method name is too long to type, and i’m lazy. function processResponseHeadersAndDefineOutput($response) { ... } more then one responsibility? Tuesday, February 28, 12
91. function getPage($data) { ... } function startProcess() { ... } $tr->process(“site.login”); Tuesday, February 28, 12
92. function getPage($data) { ... } get from where? function startProcess() { ... } $tr->process(“site.login”); Tuesday, February 28, 12
93. Use clearer names: fetchPage() downloadPage() function getPage($data) { ... } get from where? function startProcess() { ... } $tr->process(“site.login”); Tuesday, February 28, 12
94. Use clearer names: fetchPage() downloadPage() function getPage($data) { ... } get from where? function startProcess() { ... } Use a thesaurus: fork, create, begin, open $tr->process(“site.login”); Tuesday, February 28, 12
95. Use clearer names: fetchPage() downloadPage() function getPage($data) { ... } get from where? function startProcess() { ... } Use a thesaurus: fork, create, begin, open $tr->process(“site.login”); Table row? Tuesday, February 28, 12
96. Use clearer names: fetchPage() downloadPage() function getPage($data) { ... } get from where? function startProcess() { ... } Use a thesaurus: fork, create, begin, open $tr->process(“site.login”); Easy understanding, complete scope: $translatorService Table row? Tuesday, February 28, 12
97. Key Benefits •Clearer communication •Indicates underlying problems Tuesday, February 28, 12
98. OC #6 “Keep your classes small” Adapted Tuesday, February 28, 12
99. 100 lines per class 15 classes per package Tuesday, February 28, 12
100. 100 lines per class 15 classes per package Increased to include docblocks Tuesday, February 28, 12
101. 100 lines per class 15 classes per package 15-20 lines per method Increased to include docblocks Tuesday, February 28, 12
102. 100 lines per class 15 classes per package 15-20 lines per method Increased to include docblocks read this as namespace or folder Tuesday, February 28, 12
103. Key Benefits •Single Responsibility •Objective methods •Slimmer namespaces •Less clunky folders Tuesday, February 28, 12
104. OC #7 “Limit the number of instance variables in a class (max: 2 5)” Adapted Tuesday, February 28, 12
105. class MyRegistrationService { protected $userService; protected $passwordService; protected $logger; protected $translator; protected $entityManager; protected $imageCropper; // ... } Tuesday, February 28, 12
106. class MyRegistrationService { protected $userService; protected $passwordService; protected $logger; protected $translator; protected $entityManager; protected $imageCropper; // ... } Limit: 5 Tuesday, February 28, 12
107. class MyRegistrationService { protected $userService; protected $passwordService; protected $logger; protected $translator; protected $entityManager; protected $imageCropper; // ... } Limit: 5 Use and event based system and move this to listener All DB interaction should be in userService Tuesday, February 28, 12
108. Key Benefits •Shorter dependency list •Easier Mocking for unit test Tuesday, February 28, 12
109. OC #8 “Use first class collections” Tuesday, February 28, 12
110. $collection->getIterator(); $collection->filter(...); $collection->append(...); $collection->map(...); Doctrine: ArrayCollection Tuesday, February 28, 12
111. Key Benefits •Implements collection operations •Uses SPL interfaces Tuesday, February 28, 12
112. OC #9 “Do not use accessors (getter/setter)” Dropped Tuesday, February 28, 12
113. /** * THIS CLASS WAS GENERATED BY THE DOCTRINE ORM. DO NOT EDIT THIS FILE. */ class DoctrineTestsModelsCMSCmsUserProxy extends \Doctrine\Tests\Models\CMS\CmsUser implements \Doctrine\ORM\Proxy\Proxy { public function getId() { $this->__load(); return parent::getId(); } public function getStatus() { $this->__load(); return parent::getStatus(); } Tuesday, February 28, 12
114. /** * THIS CLASS WAS GENERATED BY THE DOCTRINE ORM. DO NOT EDIT THIS FILE. */ class DoctrineTestsModelsCMSCmsUserProxy extends \Doctrine\Tests\Models\CMS\CmsUser implements \Doctrine\ORM\Proxy\Proxy { public function getId() { $this->__load(); return parent::getId(); } public function getStatus() { $this->__load(); return parent::getStatus(); } Example: Doctrine uses getters to inject lazy loading operations Tuesday, February 28, 12
115. Key Benefits •Injector operations •Encapsulation of transformations Tuesday, February 28, 12
116. OC #10 (bonus!) “Document your code!” Created! Tuesday, February 28, 12
117. //check to see if the section above set the $overall_pref variable to void if ($overall_pref == 'void') // implode the revised array of selections in group three into a string // variable so that it can be transferred to the database at the end of the // page $groupthree = implode($groupthree_array, "\n\r"); Tuesday, February 28, 12
118. //check to see if the section above set the $overall_pref variable to void if ($overall_pref == 'void') really? // implode the revised array of selections in group three into a string // variable so that it can be transferred to the database at the end of the // page $groupthree = implode($groupthree_array, "\n\r"); Tuesday, February 28, 12
119. //check to see if the section above set the $overall_pref variable to void if ($overall_pref == 'void') really? // implode the revised array of selections in group three into a string // variable so that it can be transferred to the database at the end of the // page $groupthree = implode($groupthree_array, "\n\r"); Documenting because i’m doing it wrong in an anusual way Tuesday, February 28, 12
120. $priority = isset($event['priority']) ? $event['priority'] : 0; if (!isset($event['event'])) { throw new \InvalidArgumentException(...)); } if (!isset($event['method'])) { $event['method'] = 'on'.preg_replace(array( '/(?<=\b)[a-z]/ie', '/[^a-z0-9]/i' ), array('strtoupper("\\0")', ''), $event['event']); } $definition->addMethodCall( 'addListenerService', array($event['event'], array($listenerId, $event['method']), $priority )); Source: Symfony2 Tuesday, February 28, 12
121. $priority = isset($event['priority']) ? $event['priority'] : 0; if (!isset($event['event'])) { throw new \InvalidArgumentException(...)); } if (!isset($event['method'])) { $event['method'] = 'on'.preg_replace(array( '/(?<=\b)[a-z]/ie', '/[^a-z0-9]/i' ), array('strtoupper("\\0")', ''), $event['event']); } $definition->addMethodCall( 'addListenerService', array($event['event'], array($listenerId, $event['method']), $priority )); What does this do? Source: Symfony2 Tuesday, February 28, 12
122. $priority = isset($event['priority']) ? $event['priority'] : 0; if (!isset($event['event'])) { throw new \InvalidArgumentException(...)); } if (!isset($event['method'])) { $event['method'] = 'on'.preg_replace(array( '/(?<=\b)[a-z]/ie', '/[^a-z0-9]/i' ), array('strtoupper("\\0")', ''), $event['event']); } $definition->addMethodCall( 'addListenerService', array($event['event'], array($listenerId, $event['method']), $priority )); What does this do? Add a simple comment: //Strips special chars and camel cases to onXxx Source: Symfony2 Tuesday, February 28, 12
123. $priority = isset($event['priority']) ? $event['priority'] : 0; if (!isset($event['event'])) { throw new \InvalidArgumentException(...)); } if (!isset($event['method'])) { $event['method'] = 'on'.preg_replace(array( '/(?<=\b)[a-z]/ie', '/[^a-z0-9]/i' ), array('strtoupper("\\0")', ''), $event['event']); } $definition->addMethodCall( 'addListenerService', array($event['event'], array($listenerId, $event['method']), $priority )); What does this do? Add a simple comment: //Strips special chars and camel cases to onXxx Source: Symfony2 Don’t explain bad code, fix it! Tuesday, February 28, 12
124. /** * Checks whether an element is contained in the collection. * This is an O(n) operation, where n is the size of the collection. * * @todo implement caching for better performance * @param mixed $element The element to search for. * @return boolean TRUE if the collection contains the element, or FALSE. */ function contains($element); Tuesday, February 28, 12
125. /** * Checks whether an element is contained in the collection. * This is an O(n) operation, where n is the size of the collection. * * @todo implement caching for better performance * @param mixed $element The element to search for. * @return boolean TRUE if the collection contains the element, or FALSE. */ function contains($element); mark todo items so the changes don’t get lost Tuesday, February 28, 12
126. /** * Checks whether an element is contained in the collection. * This is an O(n) operation, where n is the size of the collection. * * @todo implement caching for better performance * @param mixed $element The element to search for. * @return boolean TRUE if the collection contains the element, or FALSE. */ function contains($element); mark todo items so the changes don’t get lost A note on cost of running function Tuesday, February 28, 12
127. /** * Checks whether an element is contained in the collection. * This is an O(n) operation, where n is the size of the collection. * * @todo implement caching for better performance * @param mixed $element The element to search for. * @return boolean TRUE if the collection contains the element, or FALSE. */ function contains($element); mark todo items so the changes don’t get lost A note on cost of running function Do a mind dump, then clean it up. Tuesday, February 28, 12
128. /** * Checks whether an element is contained in the collection. * This is an O(n) operation, where n is the size of the collection. * * @todo implement caching for better performance * @param mixed $element The element to search for. * @return boolean TRUE if the collection contains the element, or FALSE. */ function contains($element); mark todo items so the changes don’t get lost A note on cost of running function Do a mind dump, then clean it up. Generate API docs ex: docBlox Tuesday, February 28, 12
129. Key Benefits •Automatic API documentation •Transmission of “line of thought” •Avoids confusion Tuesday, February 28, 12
130. Recap • #1 - Only one indentation level per method. • #2 - Do not use the ‘else’ keyword. • #3 - Wrap primitive types and string, if it has behavior. • #4 - Only one -> per line, if not getter or fluent. • #5 - Do not Abbreviate. • #6 - Keep your classes small • #7 - Limit the number of instance variables in a class (max: 5) • #8 - Use first class collections • #9 - Use accessors (getter/setter) • #10 - Document your code! Tuesday, February 28, 12
131. Questions? @rdohms http://slides.doh.ms http://doh.ms rafael @doh.ms https://joind.in/6125 Tuesday, February 28, 12
132. Recommended Links: http://goo.gl/OcSNx http://goo.gl/unrij DISCLAIMER: This talk re-uses some of the examples used by Guilherme Blanco in his original Object Calisthenic talk. These principles were studied and applied by us while we worked together in previous jobs. The result taught us all a lesson we really want to spread to other developers. The ThoughtWorks Anthology The Art of Readable Code https://joind.in/6125 Tuesday, February 28, 12