From 731b9df4ab6f8389fe90df159c5d1788dcdb5388 Mon Sep 17 00:00:00 2001 From: Rangga K D <36787606+ranggakd@users.noreply.github.com> Date: Wed, 31 Aug 2022 08:04:10 +0700 Subject: [PATCH 01/18] Update README.md first step into Indonesian translation --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b5b80b0e..7e151d10 100644 --- a/README.md +++ b/README.md @@ -2205,5 +2205,7 @@ This is also available in other languages: * [ahmedalmory/clean-code-php](https://github.com/ahmedalmory/clean-code-php) * :jp: **Japanese:** * [hayato07/clean-code-php](https://github.com/hayato07/clean-code-php) - +* :ina: **Indonesia:** + * [ranggakd/clean-code-php](https://github.com/ranggakd/clean-code-php) + **[⬆ back to top](#table-of-contents)** From b9c59725bf6aca27266178cbc562e7d36a296db8 Mon Sep 17 00:00:00 2001 From: Rangga K D <36787606+ranggakd@users.noreply.github.com> Date: Wed, 31 Aug 2022 08:05:09 +0700 Subject: [PATCH 02/18] Update README.md indonesia emoji --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7e151d10..fac002e2 100644 --- a/README.md +++ b/README.md @@ -2205,7 +2205,7 @@ This is also available in other languages: * [ahmedalmory/clean-code-php](https://github.com/ahmedalmory/clean-code-php) * :jp: **Japanese:** * [hayato07/clean-code-php](https://github.com/hayato07/clean-code-php) -* :ina: **Indonesia:** +* :indonesia: **Indonesia:** * [ranggakd/clean-code-php](https://github.com/ranggakd/clean-code-php) **[⬆ back to top](#table-of-contents)** From 84c15da02a7c88c4375cce59d289577c957e33a2 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Sun, 11 Sep 2022 22:30:34 +0700 Subject: [PATCH 03/18] minor indonesian translation fixed --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index fac002e2..de88aa03 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Clean Code PHP +# Clean Code PHP ## Table of Contents @@ -2205,7 +2205,7 @@ This is also available in other languages: * [ahmedalmory/clean-code-php](https://github.com/ahmedalmory/clean-code-php) * :jp: **Japanese:** * [hayato07/clean-code-php](https://github.com/hayato07/clean-code-php) -* :indonesia: **Indonesia:** +* :indonesia: **Indonesian:** * [ranggakd/clean-code-php](https://github.com/ranggakd/clean-code-php) **[⬆ back to top](#table-of-contents)** From c2a85ba22542231b8e0dea423a5e070899ee5018 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Sun, 11 Sep 2022 22:46:01 +0700 Subject: [PATCH 04/18] grammar error checking by grammarly ext in vsc --- README.md | 63 +++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index de88aa03..d866b438 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ * [Use identical comparison](#use-identical-comparison) * [Null coalescing operator](#null-coalescing-operator) 4. [Functions](#functions) - * [Use default arguments instead of short circuiting or conditionals](#use-default-arguments-instead-of-short-circuiting-or-conditionals) + * [Use default arguments instead of short-circuiting or conditionals](#use-default-arguments-instead-of-short-circuiting-or-conditionals) * [Function arguments (2 or fewer ideally)](#function-arguments-2-or-fewer-ideally) * [Function names should say what they do](#function-names-should-say-what-they-do) * [Functions should only be one level of abstraction](#functions-should-only-be-one-level-of-abstraction) @@ -58,7 +58,7 @@ Not every principle herein has to be strictly followed, and even fewer will be u agreed upon. These are guidelines and nothing more, but they are ones codified over many years of collective experience by the authors of *Clean Code*. -Inspired from [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript). +Inspired by [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript). Although many developers still use PHP 5, most of the examples in this article only work with PHP 7.1+. @@ -404,7 +404,7 @@ The comparison `$a !== $b` returns `TRUE`. ### Null coalescing operator -Null coalescing is a new operator [introduced in PHP 7](https://www.php.net/manual/en/migration70.new-features.php). The null coalescing operator `??` has been added as syntactic sugar for the common case of needing to use a ternary in conjunction with `isset()`. It returns its first operand if it exists and is not `null`; otherwise it returns its second operand. +Null coalescing is a new operator [introduced in PHP 7](https://www.php.net/manual/en/migration70.new-features.php). The null coalescing operator `??` has been added as syntactic sugar for the common case of needing to use a ternary in conjunction with `isset()`. It returns its first operand if it exists and is not `null`; otherwise, it returns its second operand. **Bad:** @@ -427,7 +427,7 @@ $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ## Functions -### Use default arguments instead of short circuiting or conditionals +### Use default arguments instead of short-circuiting or conditionals **Not good:** @@ -467,11 +467,11 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void ### Function arguments (2 or fewer ideally) -Limiting the amount of function parameters is incredibly important because it makes +Limiting the number of function parameters is incredibly important because it makes testing your function easier. Having more than three leads to a combinatorial explosion where you have to test tons of different cases with each separate argument. -Zero arguments is the ideal case. One or two arguments is ok, and three should be avoided. +Zero arguments are the ideal case. One or two arguments are ok, and three should be avoided. Anything more than that should be consolidated. Usually, if you have more than two arguments then your function is trying to do too much. In cases where it's not, most of the time a higher-level object will suffice as an argument. @@ -679,7 +679,7 @@ function parseBetterPHPAlternative(string $code): void **Good:** -The best solution is move out the dependencies of `parseBetterPHPAlternative()` function. +The best solution is to move out the dependencies of `parseBetterPHPAlternative()` function. ```php class Tokenizer @@ -785,7 +785,7 @@ example, you might need to write to a file. What you want to do is to centralize you are doing this. Don't have several functions and classes that write to a particular file. Have one service that does it. One and only one. -The main point is to avoid common pitfalls like sharing state between objects without +The main point is to avoid common pitfalls like sharing states between objects without any structure, using mutable data types that can be written to by anything, and not centralizing where your side effects occur. If you can do this, you will be happier than the vast majority of other programmers. @@ -834,8 +834,8 @@ var_dump($newName); Polluting globals is a bad practice in many languages because you could clash with another library and the user of your API would be none-the-wiser until they get an exception in -production. Let's think about an example: what if you wanted to have configuration array? -You could write global function like `config()`, but it could clash with another library +production. Let's think about an example: what if you wanted to have a configuration array? +You could write a global function like `config()`, but it could clash with another library that tried to do the same thing. **Bad:** @@ -869,7 +869,7 @@ class Configuration } ``` -Load configuration and create instance of `Configuration` class +Load configuration and create an instance of `Configuration`` class ```php $configuration = new Configuration([ @@ -877,7 +877,7 @@ $configuration = new Configuration([ ]); ``` -And now you must use instance of `Configuration` in your application. +And now you must use the instance of `Configuration`` in your application. **[⬆ back to top](#table-of-contents)** @@ -887,9 +887,9 @@ Singleton is an [anti-pattern](https://en.wikipedia.org/wiki/Singleton_pattern). 1. They are generally used as a **global instance**, why is that so bad? Because **you hide the dependencies** of your application in your code, instead of exposing them through the interfaces. Making something global to avoid passing it around is a [code smell](https://en.wikipedia.org/wiki/Code_smell). 2. They violate the [single responsibility principle](#single-responsibility-principle-srp): by virtue of the fact that **they control their own creation and lifecycle**. 3. They inherently cause code to be tightly [coupled](https://en.wikipedia.org/wiki/Coupling_%28computer_programming%29). This makes faking them out under **test rather difficult** in many cases. - 4. They carry state around for the lifetime of the application. Another hit to testing since **you can end up with a situation where tests need to be ordered** which is a big no for unit tests. Why? Because each unit test should be independent from the other. + 4. They carry state around for the lifetime of the application. Another hit to testing since **you can end up with a situation where tests need to be ordered** which is a big no for unit tests. Why? Because each unit test should be independent of the other. -There is also very good thoughts by [Misko Hevery](http://misko.hevery.com/about/) about the [root of problem](http://misko.hevery.com/2008/08/25/root-cause-of-singletons/). +There are also very good thoughts by [Misko Hevery](http://misko.hevery.com/about/) about the [root of the problem](http://misko.hevery.com/2008/08/25/root-cause-of-singletons/). **Bad:** @@ -932,13 +932,13 @@ class DBConnection } ``` -Create instance of `DBConnection` class and configure it with [DSN](http://php.net/manual/en/pdo.construct.php#refsect1-pdo.construct-parameters). +Create an instance of `DBConnection` class and configure it with [DSN](http://php.net/manual/en/pdo.construct.php#refsect1-pdo.construct-parameters). ```php $connection = new DBConnection($dsn); ``` -And now you must use instance of `DBConnection` in your application. +And now you must use the instance of `DBConnection` in your application. **[⬆ back to top](#table-of-contents)** @@ -1249,13 +1249,13 @@ $balance = $bankAccount->getBalance(); ### Make objects have private/protected members -* `public` methods and properties are most dangerous for changes, because some outside code may easily rely on them and you can't control what code relies on them. **Modifications in class are dangerous for all users of class.** -* `protected` modifier are as dangerous as public, because they are available in scope of any child class. This effectively means that difference between public and protected is only in access mechanism, but encapsulation guarantee remains the same. **Modifications in class are dangerous for all descendant classes.** -* `private` modifier guarantees that code is **dangerous to modify only in boundaries of single class** (you are safe for modifications and you won't have [Jenga effect](http://www.urbandictionary.com/define.php?term=Jengaphobia&defid=2494196)). +* `public` methods and properties are most dangerous for changes because some outside code may easily rely on them and you can't control what code relies on them. **Modifications in class are dangerous for all users of the class.** +* `protected` modifiers are as dangerous as public because they are available in the scope of any child class. This effectively means that difference between public and protected is only in the access mechanism, but the encapsulation guarantee remains the same. **Modifications in class are dangerous for all descendant classes.** +* `private` modifier guarantees that code is **dangerous to modify only within the boundaries of a single class** (you are safe for modifications and you won't have [Jenga effect](http://www.urbandictionary.com/define.php?term=Jengaphobia&defid=2494196)). -Therefore, use `private` by default and `public/protected` when you need to provide access for external classes. +Therefore, use `private` by default and `public/protected` when you need to provide access to external classes. -For more information you can read the [blog post](http://fabien.potencier.org/pragmatism-over-theory-protected-vs-private.html) on this topic written by [Fabien Potencier](https://github.com/fabpot). +For more information, you can read the [blog post](http://fabien.potencier.org/pragmatism-over-theory-protected-vs-private.html) on this topic written by [Fabien Potencier](https://github.com/fabpot). **Bad:** @@ -1307,9 +1307,9 @@ echo 'Employee name: ' . $employee->getName(); As stated famously in [*Design Patterns*](https://en.wikipedia.org/wiki/Design_Patterns) by the Gang of Four, you should prefer composition over inheritance where you can. There are lots of good reasons to use inheritance and lots of good reasons to use composition. -The main point for this maxim is that if your mind instinctively goes for +The main point of this maxim is that if your mind instinctively goes for an inheritance, try to think if composition could model your problem better. In some -cases it can. +cases, it can. You might be wondering then, "when should I use inheritance?" It depends on your problem at hand, but this is a decent list of when inheritance @@ -1405,8 +1405,7 @@ class Employee ### Avoid fluent interfaces -A [Fluent interface](https://en.wikipedia.org/wiki/Fluent_interface) is an object -oriented API that aims to improve the readability of the source code by using +A [Fluent interface](https://en.wikipedia.org/wiki/Fluent_interface) is an object-oriented API that aims to improve the readability of the source code by using [Method chaining](https://en.wikipedia.org/wiki/Method_chaining). While there can be some contexts, frequently builder objects, where this @@ -1417,9 +1416,9 @@ more often it comes at some costs: 1. Breaks [Encapsulation](https://en.wikipedia.org/wiki/Encapsulation_%28object-oriented_programming%29). 2. Breaks [Decorators](https://en.wikipedia.org/wiki/Decorator_pattern). 3. Is harder to [mock](https://en.wikipedia.org/wiki/Mock_object) in a test suite. -4. Makes diffs of commits harder to read. +4. Makes the diffs in commits harder to read. -For more information you can read the full [blog post](https://ocramius.github.io/blog/fluent-interfaces-are-evil/) +For more information, you can read the full [blog post](https://ocramius.github.io/blog/fluent-interfaces-are-evil/) on this topic written by [Marco Pivetta](https://github.com/Ocramius). **Bad:** @@ -1523,7 +1522,7 @@ The `final` keyword should be used whenever possible: The only condition is that your class should implement an interface and no other public methods are defined. -For more informations you can read [the blog post](https://ocramius.github.io/blog/when-to-declare-classes-final/) on this topic written by [Marco Pivetta (Ocramius)](https://ocramius.github.io/). +For more information, you can read [the blog post](https://ocramius.github.io/blog/when-to-declare-classes-final/) on this topic written by [Marco Pivetta (Ocramius)](https://ocramius.github.io/). **Bad:** @@ -1592,7 +1591,7 @@ As stated in Clean Code, "There should never be more than one reason for a class to change". It's tempting to jam-pack a class with a lot of functionality, like when you can only take one suitcase on your flight. The issue with this is that your class won't be conceptually cohesive and it will give it many reasons -to change. Minimizing the amount of times you need to change a class is important. +to change. Minimizing the number of times you need to change a class is important. It's important because if too much functionality is in one class and you modify a piece of it, it can be difficult to understand how that will affect other dependent modules in your codebase. @@ -2008,7 +2007,7 @@ abstractions. This can be hard to understand at first, but if you've worked with PHP frameworks (like Symfony), you've seen an implementation of this principle in the form of Dependency Injection (DI). While they are not identical concepts, DIP keeps high-level -modules from knowing the details of its low-level modules and setting them up. +modules from knowing the details of their low-level modules and setting them up. It can accomplish this through DI. A huge benefit of this is that it reduces the coupling between modules. Coupling is a very bad development pattern because it makes your code hard to refactor. @@ -2101,7 +2100,7 @@ change some logic. Imagine if you run a restaurant and you keep track of your inventory: all your tomatoes, onions, garlic, spices, etc. If you have multiple lists that you keep this on, then all have to be updated when you serve a dish with -tomatoes in them. If you only have one list, there's only one place to update! +tomatoes in them. If you only have one list, there's only one place to update it! Often you have duplicate code because you have two or more slightly different things, that share a lot in common, but their differences force you @@ -2112,7 +2111,7 @@ things with just one function/module/class. Getting the abstraction right is critical, that's why you should follow the SOLID principles laid out in the [Classes](#classes) section. Bad abstractions can be worse than duplicate code, so be careful! Having said this, if you can make -a good abstraction, do it! Don't repeat yourself, otherwise you'll find yourself +a good abstraction, do it! Don't repeat yourself, otherwise, you'll find yourself updating multiple places any time you want to change one thing. **Bad:** From 79173e00831ffbea64ce63a5c1cf67bad23f3d30 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Tue, 13 Sep 2022 22:42:05 +0700 Subject: [PATCH 05/18] minor back-to-top section added --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index d866b438..a986e97b 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,8 @@ Inspired by [clean-code-javascript](https://github.com/ryanmcdermott/clean-code- Although many developers still use PHP 5, most of the examples in this article only work with PHP 7.1+. +**[⬆ back to top](#table-of-contents)** + ## Variables ### Use meaningful and pronounceable variable names @@ -119,6 +121,8 @@ $result = $serializer->serialize($data, 448); $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); ``` +**[⬆ back to top](#table-of-contents)** + ### Use searchable names (part 2) **Bad:** @@ -1585,6 +1589,8 @@ final class Car implements Vehicle * [I: Interface Segregation Principle (ISP)](#interface-segregation-principle-isp) * [D: Dependency Inversion Principle (DIP)](#dependency-inversion-principle-dip) +**[⬆ back to top](#table-of-contents)** + ### Single Responsibility Principle (SRP) As stated in Clean Code, "There should never be more than one reason for a class From b8a3381945a9ace97412644d08c6d1f173dd3ac9 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Tue, 13 Sep 2022 22:58:58 +0700 Subject: [PATCH 06/18] provide refactor for better diff comparison p1 --- README.md | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/README.md b/README.md index a986e97b..710863ce 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,13 @@ $ymdstr = $moment->format('y-m-d'); $currentDate = $moment->format('y-m-d'); ``` +**Refactor:** + +```diff +- $ymdstr = $moment->format('y-m-d'); ++ $currentDate = $moment->format('y-m-d'); +``` + **[⬆ back to top](#table-of-contents)** ### Use the same vocabulary for the same type of variable @@ -99,6 +106,16 @@ getUserProfile(); getUser(); ``` +**Refactor:** + +```diff +- getUserInfo(); +- getUserData(); +- getUserRecord(); +- getUserProfile(); ++ getUser(); +``` + **[⬆ back to top](#table-of-contents)** ### Use searchable names (part 1) @@ -121,6 +138,13 @@ $result = $serializer->serialize($data, 448); $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); ``` +**Refactor:** + +```diff +- $result = $serializer->serialize($data, 448); ++ $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); +``` + **[⬆ back to top](#table-of-contents)** ### Use searchable names (part 2) @@ -168,6 +192,31 @@ if ($user->access & User::ACCESS_UPDATE) { $user->access ^= User::ACCESS_CREATE; ``` +**Refactor:** + +```diff +class User +{ +- public $access = 7; ++ public const ACCESS_READ = 1; ++ ++ public const ACCESS_CREATE = 2; ++ ++ public const ACCESS_UPDATE = 4; ++ ++ public const ACCESS_DELETE = 8; ++ ++ public $access = self::ACCESS_READ | self::ACCESS_CREATE | self::ACCESS_UPDATE; +} + +- if ($user->access & 4) { ++ if ($user->access & User::ACCESS_UPDATE) { +} + +- $user->access ^= 2; ++ $user->access ^= User::ACCESS_CREATE; +``` + **[⬆ back to top](#table-of-contents)** ### Use explanatory variables @@ -207,6 +256,20 @@ preg_match($cityZipCodeRegex, $address, $matches); saveCityZipCode($matches['city'], $matches['zipCode']); ``` +**Refactor:** + +```diff +$address = 'One Infinite Loop, Cupertino 95014'; +- $cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/'; ++ $cityZipCodeRegex = '/^[^,]+,\s*(?.+?)\s*(?\d{5})$/'; +preg_match($cityZipCodeRegex, $address, $matches); + +- saveCityZipCode($matches[1], $matches[2]); +- [, $city, $zipCode] = $matches; +- saveCityZipCode($city, $zipCode); ++ saveCityZipCode($matches['city'], $matches['zipCode']); +``` + **[⬆ back to top](#table-of-contents)** ### Avoid nesting too deeply and return early (part 1) @@ -252,6 +315,36 @@ function isShopOpen(string $day): bool } ``` +**Refactor:** + +```diff +function isShopOpen(string $day): bool +{ +- if ($day) { +- if (is_string($day)) { +- $day = strtolower($day); +- if ($day === 'friday') { +- return true; +- } elseif ($day === 'saturday') { +- return true; +- } elseif ($day === 'sunday') { +- return true; +- } +- return false; +- } +- return false; +- } +- return false; ++ if (empty($day)) { ++ return false; ++ } ++ ++ $openingDays = ['friday', 'saturday', 'sunday']; ++ ++ return in_array(strtolower($day), $openingDays, true); +} +``` + **[⬆ back to top](#table-of-contents)** ### Avoid nesting too deeply and return early (part 2) @@ -291,6 +384,34 @@ function fibonacci(int $n): int } ``` +**Refactor:** + +```diff +- function fibonacci(int $n) ++ function fibonacci(int $n): int +{ +- if ($n < 50) { +- if ($n !== 0) { +- if ($n !== 1) { +- return fibonacci($n - 1) + fibonacci($n - 2); +- } +- return 1; +- } +- return 0; +- } +- return 'Not supported'; ++ if ($n === 0 || $n === 1) { ++ return $n; ++ } ++ ++ if ($n >= 50) { ++ throw new Exception('Not supported'); ++ } ++ ++ return fibonacci($n - 1) + fibonacci($n - 2); +} +``` + **[⬆ back to top](#table-of-contents)** ### Avoid Mental Mapping From 5bac123609be67a9b2ba31dd326ebcc8d5850981 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Wed, 14 Sep 2022 22:32:21 +0700 Subject: [PATCH 07/18] provide refactor for better diff comparison p2 --- README.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/README.md b/README.md index 710863ce..a0a56d9c 100644 --- a/README.md +++ b/README.md @@ -451,6 +451,25 @@ foreach ($locations as $location) { } ``` +**Refactor:** + +```diff +- $l = ['Austin', 'New York', 'San Francisco']; ++ $locations = ['Austin', 'New York', 'San Francisco']; + +- for ($i = 0; $i < count($l); $i++) { +- $li = $l[$i]; ++ foreach ($locations as $location) { + doStuff(); + doSomeOtherStuff(); + // ... + // ... + // ... +- dispatch($li); ++ dispatch($location); +} +``` + **[⬆ back to top](#table-of-contents)** ### Don't add unneeded context @@ -488,6 +507,24 @@ class Car } ``` +**Refactor:** + +```diff +class Car +{ +- public $carMake; ++ public $make; + +- public $carModel; ++ public $model; + +- public $carColor; ++ public $color; + + //... +} +``` + **[⬆ back to top](#table-of-contents)** ## Comparison From 453688e656b3609735ed6f849c1035a15f9f3b27 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Sun, 18 Sep 2022 23:03:46 +0700 Subject: [PATCH 08/18] minor grammar errors fixed --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index a0a56d9c..bc6f0e5f 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ readable, reusable, and refactorable software in PHP. Not every principle herein has to be strictly followed, and even fewer will be universally agreed upon. These are guidelines and nothing more, but they are ones codified over many -years of collective experience by the authors of *Clean Code*. +years of collective experience by the authors of the *Clean Code*. Inspired by [clean-code-javascript](https://github.com/ryanmcdermott/clean-code-javascript). @@ -545,7 +545,7 @@ if ($a != $b) { ``` The comparison `$a != $b` returns `FALSE` but in fact it's `TRUE`! -The string `42` is different than the integer `42`. +The string `42` is different from the integer `42`. **Good:** @@ -841,7 +841,7 @@ function parseBetterPHPAlternative(string $code): void **Good:** -The best solution is to move out the dependencies of `parseBetterPHPAlternative()` function. +The best solution is to move out the dependencies of the `parseBetterPHPAlternative()` function. ```php class Tokenizer @@ -1031,7 +1031,7 @@ class Configuration } ``` -Load configuration and create an instance of `Configuration`` class +Load configuration and create an instance of the `Configuration` class ```php $configuration = new Configuration([ @@ -1039,7 +1039,7 @@ $configuration = new Configuration([ ]); ``` -And now you must use the instance of `Configuration`` in your application. +And now you must use the instance of `Configuration` in your application. **[⬆ back to top](#table-of-contents)** @@ -1350,7 +1350,7 @@ to look up and change every accessor in your codebase. * You can lazy load your object's properties, let's say getting it from a server. -Additionally, this is part of [Open/Closed](#openclosed-principle-ocp) principle. +Additionally, this is part of the [Open/Closed](#openclosed-principle-ocp) principle. **Bad:** @@ -1413,7 +1413,7 @@ $balance = $bankAccount->getBalance(); * `public` methods and properties are most dangerous for changes because some outside code may easily rely on them and you can't control what code relies on them. **Modifications in class are dangerous for all users of the class.** * `protected` modifiers are as dangerous as public because they are available in the scope of any child class. This effectively means that difference between public and protected is only in the access mechanism, but the encapsulation guarantee remains the same. **Modifications in class are dangerous for all descendant classes.** -* `private` modifier guarantees that code is **dangerous to modify only within the boundaries of a single class** (you are safe for modifications and you won't have [Jenga effect](http://www.urbandictionary.com/define.php?term=Jengaphobia&defid=2494196)). +* `private` modifier guarantees that code is **dangerous to modify only within the boundaries of a single class** (you are safe for modifications and you won't have the [Jenga effect](http://www.urbandictionary.com/define.php?term=Jengaphobia&defid=2494196)). Therefore, use `private` by default and `public/protected` when you need to provide access to external classes. From 15d1809e0554ad9c75bbf050ddab53e48ede57c3 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Sun, 18 Sep 2022 23:17:23 +0700 Subject: [PATCH 09/18] minor refactor code diff fixed --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index bc6f0e5f..968bd80e 100644 --- a/README.md +++ b/README.md @@ -318,7 +318,8 @@ function isShopOpen(string $day): bool **Refactor:** ```diff -function isShopOpen(string $day): bool +- function isShopOpen($day): bool ++ function isShopOpen(string $day): bool { - if ($day) { - if (is_string($day)) { From 193fee0bf868340b2d611576fbc0e98bf214b9d2 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 29 Sep 2022 21:09:00 +0700 Subject: [PATCH 10/18] adding refactor code 'til middle function section --- README.md | 193 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) diff --git a/README.md b/README.md index 968bd80e..08f04874 100644 --- a/README.md +++ b/README.md @@ -561,6 +561,18 @@ if ($a !== $b) { } ``` +**Refactor:** + +```diff +$a = '42'; +$b = 42; + +- if ($a != $b) { ++ if ($a !== $b) { + // +} +``` + The comparison `$a !== $b` returns `TRUE`. **[⬆ back to top](#table-of-contents)** @@ -586,6 +598,19 @@ if (isset($_GET['name'])) { $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ``` +**Refactor:** + +```diff +- if (isset($_GET['name'])) { +- $name = $_GET['name']; +- } elseif (isset($_POST['name'])) { +- $name = $_POST['name']; +- } else { +- $name = 'nobody'; +-} ++ $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; +``` + **[⬆ back to top](#table-of-contents)** ## Functions @@ -626,6 +651,18 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void } ``` +**Refactor:** + +```diff +- function createMicrobrewery($breweryName = 'Hipster Brew Co.'): void +- function createMicrobrewery($name = null): void ++ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void +{ +- $breweryName = $name ?: 'Hipster Brew Co.'; + // ... +} +``` + **[⬆ back to top](#table-of-contents)** ### Function arguments (2 or fewer ideally) @@ -722,6 +759,79 @@ class Questionnaire } ``` +**Refactor:** + +```diff ++class Name ++{ ++ private $firstname; ++ ++ private $lastname; ++ ++ private $patronymic; ++ ++ public function __construct(string $firstname, string $lastname, string $patronymic) ++ { ++ $this->firstname = $firstname; ++ $this->lastname = $lastname; ++ $this->patronymic = $patronymic; ++ } ++ ++ // getters ... ++} ++ ++class City ++{ ++ private $region; ++ ++ private $district; ++ ++ private $city; ++ ++ public function __construct(string $region, string $district, string $city) ++ { ++ $this->region = $region; ++ $this->district = $district; ++ $this->city = $city; ++ } ++ ++ // getters ... ++} ++ ++class Contact ++{ ++ private $phone; ++ ++ private $email; ++ ++ public function __construct(string $phone, string $email) ++ { ++ $this->phone = $phone; ++ $this->email = $email; ++ } ++ ++ // getters ... ++} ++ +class Questionnaire +{ +- public function __construct( +- string $firstname, +- string $lastname, +- string $patronymic, +- string $region, +- string $district, +- string $city, +- string $phone, +- string $email +- ) { ++ public function __construct(Name $name, City $city, Contact $contact) ++ { + // ... + } +} +``` + **[⬆ back to top](#table-of-contents)** ### Function names should say what they do @@ -762,6 +872,26 @@ $message = new Email(...); $message->send(); ``` +**Refactor:** + +```diff +class Email +{ + //... + +- public function handle(): void ++ public function send(): void + { + mail($this->to, $this->subject, $this->body); + } +} + +$message = new Email(...); +// +- $message->handle(); ++ $message->send(); +``` + **[⬆ back to top](#table-of-contents)** ### Functions should only be one level of abstraction @@ -900,6 +1030,69 @@ class BetterPHPAlternative } ``` +**Refactor:** + +```diff ++ class Tokenizer ++ { +- function tokenize(string $code): array ++ public function tokenize(string $code): array ++ { + $regexes = [ + // ... + ]; + + $statements = explode(' ', $code); + $tokens = []; + foreach ($regexes as $regex) { + foreach ($statements as $statement) { ++ $tokens[] = /* ... */; + } + } + ++ return $tokens; ++ } ++ } + ++ class Lexer ++ { +- function lexer(array $tokens): array ++ public function lexify(array $tokens): array ++ { + $ast = []; + foreach ($tokens as $token) { ++ $ast[] = /* ... */; + } + ++ return $ast; ++ } ++ } + +- function parseBetterPHPAlternative(string $code): void ++ class BetterPHPAlternative ++ { ++ private $tokenizer; ++ private $lexer; ++ ++ public function __construct(Tokenizer $tokenizer, Lexer $lexer) ++ { ++ $this->tokenizer = $tokenizer; ++ $this->lexer = $lexer; ++ } ++ ++ public function parse(string $code): void ++ { +- $tokens = tokenize($code); +- $ast = lexer($tokens); ++ $tokens = $this->tokenizer->tokenize($code); ++ $ast = $this->lexer->lexify($tokens); + foreach ($ast as $node) { + // parse... + } ++ } ++ } +``` + **[⬆ back to top](#table-of-contents)** ### Don't use flags as function parameters From bb9f1886eb2a4d4f832b49366ffafcc0f74659d2 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 29 Sep 2022 21:59:06 +0700 Subject: [PATCH 11/18] adding refactor til global func section --- README.md | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/README.md b/README.md index 08f04874..a979268c 100644 --- a/README.md +++ b/README.md @@ -1128,6 +1128,28 @@ function createTempFile(string $name): void } ``` +**Refactor:** + +```diff +- function createFile(string $name, bool $temp = false): void +- { +- if ($temp) { +- touch('./temp/' . $name); +- } else { +- touch($name); +- } +-} ++ function createFile(string $name): void ++ { ++ touch($name); ++ } ++ ++ function createTempFile(string $name): void ++ { ++ touch('./temp/' . $name); ++ } +``` + **[⬆ back to top](#table-of-contents)** ### Avoid Side Effects @@ -1184,6 +1206,30 @@ var_dump($newName); // ['Ryan', 'McDermott']; ``` +**Refactor:** + +```diff +- function splitIntoFirstAndLastName(): void ++ function splitIntoFirstAndLastName(string $name): array +{ +- global $name; +- +- $name = explode(' ', $name); ++ return explode(' ', $name); +} + +$name = 'Ryan McDermott'; +- splitIntoFirstAndLastName(); ++ $newName = splitIntoFirstAndLastName($name); + +var_dump($name); +- // ['Ryan', 'McDermott']; ++ // 'Ryan McDermott'; + ++ var_dump($newName); ++ // ['Ryan', 'McDermott']; +``` + **[⬆ back to top](#table-of-contents)** ### Don't write to global functions @@ -1235,6 +1281,35 @@ $configuration = new Configuration([ And now you must use the instance of `Configuration` in your application. +**Refactor:** + +```diff ++ class Configuration ++ { ++ private $configuration = []; ++ ++ public function __construct(array $configuration) ++ { ++ $this->configuration = $configuration; ++ } ++ ++ public function get(string $key): ?string ++ { ++ // null coalescing operator ++ return $this->configuration[$key] ?? null; ++ } ++ } ++ +- function config(): array +- { +- return [ ++ $configuration = new Configuration([ + 'foo' => 'bar', +- ]; +- } ++ ]); +``` + **[⬆ back to top](#table-of-contents)** ### Don't use a Singleton pattern From d1f6b18a2b649b55a87bc2cca685eee414280d38 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 29 Sep 2022 22:05:24 +0700 Subject: [PATCH 12/18] try adding collapsible refactor code --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a979268c..026a8d4b 100644 --- a/README.md +++ b/README.md @@ -80,12 +80,16 @@ $ymdstr = $moment->format('y-m-d'); $currentDate = $moment->format('y-m-d'); ``` -**Refactor:** +
+ + Refactor: + ```diff - $ymdstr = $moment->format('y-m-d'); + $currentDate = $moment->format('y-m-d'); ``` +
**[⬆ back to top](#table-of-contents)** From 64d76612a400d89335ac88537f607ae726ab5364 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 29 Sep 2022 22:15:06 +0700 Subject: [PATCH 13/18] change strong into b tag --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 026a8d4b..ef1143d9 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ $currentDate = $moment->format('y-m-d');
- Refactor: + Refactor: ```diff From 73ad91355128c49d3b9eb9d7017a88ec43117ae7 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 29 Sep 2022 23:20:21 +0700 Subject: [PATCH 14/18] progressing collapsible refactor code --- README.md | 66 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index ef1143d9..df7560b3 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ $currentDate = $moment->format('y-m-d');
- Refactor: +Refactor: ```diff @@ -110,7 +110,10 @@ getUserProfile(); getUser(); ``` -**Refactor:** +
+ +Refactor: + ```diff - getUserInfo(); @@ -119,6 +122,7 @@ getUser(); - getUserProfile(); + getUser(); ``` +
**[⬆ back to top](#table-of-contents)** @@ -142,12 +146,16 @@ $result = $serializer->serialize($data, 448); $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); ``` -**Refactor:** +
+ +Refactor: + ```diff - $result = $serializer->serialize($data, 448); + $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); ``` +
**[⬆ back to top](#table-of-contents)** @@ -196,7 +204,10 @@ if ($user->access & User::ACCESS_UPDATE) { $user->access ^= User::ACCESS_CREATE; ``` -**Refactor:** +
+ +Refactor: + ```diff class User @@ -220,6 +231,7 @@ class User - $user->access ^= 2; + $user->access ^= User::ACCESS_CREATE; ``` +
**[⬆ back to top](#table-of-contents)** @@ -260,7 +272,10 @@ preg_match($cityZipCodeRegex, $address, $matches); saveCityZipCode($matches['city'], $matches['zipCode']); ``` -**Refactor:** +
+ +Refactor: + ```diff $address = 'One Infinite Loop, Cupertino 95014'; @@ -273,6 +288,7 @@ preg_match($cityZipCodeRegex, $address, $matches); - saveCityZipCode($city, $zipCode); + saveCityZipCode($matches['city'], $matches['zipCode']); ``` +
**[⬆ back to top](#table-of-contents)** @@ -319,7 +335,10 @@ function isShopOpen(string $day): bool } ``` -**Refactor:** +
+ +Refactor: + ```diff - function isShopOpen($day): bool @@ -349,6 +368,7 @@ function isShopOpen(string $day): bool + return in_array(strtolower($day), $openingDays, true); } ``` +
**[⬆ back to top](#table-of-contents)** @@ -389,7 +409,10 @@ function fibonacci(int $n): int } ``` -**Refactor:** +
+ +Refactor: + ```diff - function fibonacci(int $n) @@ -416,6 +439,7 @@ function fibonacci(int $n): int + return fibonacci($n - 1) + fibonacci($n - 2); } ``` +
**[⬆ back to top](#table-of-contents)** @@ -456,7 +480,10 @@ foreach ($locations as $location) { } ``` -**Refactor:** +
+ +Refactor: + ```diff - $l = ['Austin', 'New York', 'San Francisco']; @@ -474,6 +501,7 @@ foreach ($locations as $location) { + dispatch($location); } ``` +
**[⬆ back to top](#table-of-contents)** @@ -512,7 +540,10 @@ class Car } ``` -**Refactor:** +
+ +Refactor: + ```diff class Car @@ -529,6 +560,7 @@ class Car //... } ``` +
**[⬆ back to top](#table-of-contents)** @@ -565,7 +597,12 @@ if ($a !== $b) { } ``` -**Refactor:** +The comparison `$a !== $b` returns `TRUE`. + +
+ +Refactor: + ```diff $a = '42'; @@ -576,8 +613,7 @@ $b = 42; // } ``` - -The comparison `$a !== $b` returns `TRUE`. +
**[⬆ back to top](#table-of-contents)** @@ -602,7 +638,10 @@ if (isset($_GET['name'])) { $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ``` -**Refactor:** +
+ +Refactor: + ```diff - if (isset($_GET['name'])) { @@ -614,6 +653,7 @@ $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; -} + $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ``` +
**[⬆ back to top](#table-of-contents)** From 8d80994cb36f598a58a92524c6de1e303619ed26 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 3 Nov 2022 15:11:21 +0700 Subject: [PATCH 15/18] fix: partial collapsible markdown refactor section --- README.md | 42 +++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index df7560b3..9485afe4 100644 --- a/README.md +++ b/README.md @@ -695,7 +695,10 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void } ``` -**Refactor:** +
+ +Refactor: + ```diff - function createMicrobrewery($breweryName = 'Hipster Brew Co.'): void @@ -706,6 +709,7 @@ function createMicrobrewery(string $breweryName = 'Hipster Brew Co.'): void // ... } ``` +
**[⬆ back to top](#table-of-contents)** @@ -803,7 +807,10 @@ class Questionnaire } ``` -**Refactor:** +
+ +Refactor: + ```diff +class Name @@ -875,6 +882,7 @@ class Questionnaire } } ``` +
**[⬆ back to top](#table-of-contents)** @@ -916,7 +924,10 @@ $message = new Email(...); $message->send(); ``` -**Refactor:** +
+ +Refactor: + ```diff class Email @@ -935,6 +946,7 @@ $message = new Email(...); - $message->handle(); + $message->send(); ``` +
**[⬆ back to top](#table-of-contents)** @@ -1074,7 +1086,10 @@ class BetterPHPAlternative } ``` -**Refactor:** +
+ +Refactor: + ```diff + class Tokenizer @@ -1136,6 +1151,7 @@ class BetterPHPAlternative + } + } ``` +
**[⬆ back to top](#table-of-contents)** @@ -1172,7 +1188,10 @@ function createTempFile(string $name): void } ``` -**Refactor:** +
+ +Refactor: + ```diff - function createFile(string $name, bool $temp = false): void @@ -1193,6 +1212,7 @@ function createTempFile(string $name): void + touch('./temp/' . $name); + } ``` +
**[⬆ back to top](#table-of-contents)** @@ -1250,7 +1270,10 @@ var_dump($newName); // ['Ryan', 'McDermott']; ``` -**Refactor:** +
+ +Refactor: + ```diff - function splitIntoFirstAndLastName(): void @@ -1273,6 +1296,7 @@ var_dump($name); + var_dump($newName); + // ['Ryan', 'McDermott']; ``` +
**[⬆ back to top](#table-of-contents)** @@ -1325,7 +1349,10 @@ $configuration = new Configuration([ And now you must use the instance of `Configuration` in your application. -**Refactor:** +
+ +Refactor: + ```diff + class Configuration @@ -1353,6 +1380,7 @@ And now you must use the instance of `Configuration` in your application. - } + ]); ``` +
**[⬆ back to top](#table-of-contents)** From 8fb6bc185d24f89ed2874ad7b5bc2bbd218c79cc Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Wed, 23 Nov 2022 20:54:09 +0700 Subject: [PATCH 16/18] add: collapsible refactor section all done --- README.md | 783 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 781 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9485afe4..7bd0a5f7 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,8 @@ class User - if ($user->access & 4) { + if ($user->access & User::ACCESS_UPDATE) { +- // ... ++ // do edit ... } - $user->access ^= 2; @@ -497,6 +499,7 @@ foreach ($locations as $location) { // ... // ... // ... +- // Wait, what is `$li` for again? - dispatch($li); + dispatch($location); } @@ -1443,6 +1446,39 @@ $connection = new DBConnection($dsn); And now you must use the instance of `DBConnection` in your application. +
+ +Refactor: + + +```diff +class DBConnection +{ +- private static $instance; + +- private function __construct(string $dsn) ++ public function __construct(string $dsn) + { + // ... + } + +- public static function getInstance(): self +- { +- if (self::$instance === null) { +- self::$instance = new self(); +- } +- +- return self::$instance; +- } + + // ... +} + +- $singleton = DBConnection::getInstance(); ++ $connection = new DBConnection($dsn); +``` +
+ **[⬆ back to top](#table-of-contents)** ### Encapsulate conditionals @@ -1463,6 +1499,19 @@ if ($article->isPublished()) { } ``` +
+ +Refactor: + + +```diff +- if ($article->state === 'published') { ++ if ($article->isPublished()) { + // ... +} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Avoid negative conditionals @@ -1493,6 +1542,25 @@ if (isDOMNodePresent($node)) { } ``` +
+ +Refactor: + + +```diff +- function isDOMNodeNotPresent(DOMNode $node): bool ++ function isDOMNodePresent(DOMNode $node): bool +{ + // ... +} + +- if (! isDOMNodeNotPresent($node)) { ++ if (isDOMNodePresent($node)) { + // ... +} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Avoid conditionals @@ -1568,6 +1636,63 @@ class Cessna implements Airplane } ``` +
+ +Refactor: + + +```diff +- class Airplane ++ interface Airplane +{ + // ... + + public function getCruisingAltitude(): int +- { +- switch ($this->type) { +- case '777': +- return $this->getMaxAltitude() - $this->getPassengerCount(); +- case 'Air Force One': +- return $this->getMaxAltitude(); +- case 'Cessna': +- return $this->getMaxAltitude() - $this->getFuelExpenditure(); +- } +- } +} + ++ class Boeing777 implements Airplane ++ { ++ // ... + ++ public function getCruisingAltitude(): int ++ { ++ return $this->getMaxAltitude() - $this->getPassengerCount(); ++ } ++ } + ++ class AirForceOne implements Airplane ++ { ++ // ... + ++ public function getCruisingAltitude(): int ++ { ++ return $this->getMaxAltitude(); ++ } ++ } + ++ class Cessna implements Airplane ++ { ++ // ... + ++ public function getCruisingAltitude(): int ++ { ++ return $this->getMaxAltitude() - $this->getFuelExpenditure(); ++ } ++ } + +``` +
+ **[⬆ back to top](#table-of-contents)** ### Avoid type-checking (part 1) @@ -1599,6 +1724,26 @@ function travelToTexas(Vehicle $vehicle): void } ``` +
+ +Refactor: + + +```diff +- function travelToTexas($vehicle): void ++ function travelToTexas(Vehicle $vehicle): void +{ +- if ($vehicle instanceof Bicycle) { +- $vehicle->pedalTo(new Location('texas')); +- } elseif ($vehicle instanceof Car) { +- $vehicle->driveTo(new Location('texas')); +- } ++ $vehicle->travelTo(new Location('texas')); +} +``` + +
+ **[⬆ back to top](#table-of-contents)** ### Avoid type-checking (part 2) @@ -1635,6 +1780,24 @@ function combine(int $val1, int $val2): int } ``` +
+ +Refactor: + + +```diff +- function combine($val1, $val2): int ++ function combine(int $val1, int $val2): int +{ +- if (! is_numeric($val1) || ! is_numeric($val2)) { +- throw new Exception('Must be of type Number'); +- } + + return $val1 + $val2; +} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Remove dead code @@ -1672,8 +1835,30 @@ $request = requestModule($requestUrl); inventoryTracker('apples', $request, 'www.inventory-awesome.io'); ``` -**[⬆ back to top](#table-of-contents)** +
+ +Refactor: + +```diff +- function oldRequestModule(string $url): void ++ function requestModule(string $url): void +{ + // ... +} + +- function newRequestModule(string $url): void +- { +- // ... +- } + +- $request = newRequestModule($requestUrl); ++ $request = requestModule($requestUrl); +inventoryTracker('apples', $request, 'www.inventory-awesome.io'); +``` +
+ +**[⬆ back to top](#table-of-contents)** ## Objects and Data Structures @@ -1748,6 +1933,53 @@ $bankAccount->withdraw($shoesPrice); $balance = $bankAccount->getBalance(); ``` +
+ +Refactor: + + +```diff +class BankAccount +{ +- public $balance = 1000; ++ private $balance; + ++ public function __construct(int $balance = 1000) ++ { ++ $this->balance = $balance; ++ } + ++ public function withdraw(int $amount): void ++ { ++ if ($amount > $this->balance) { ++ throw new \Exception('Amount greater than available balance.'); ++ } + ++ $this->balance -= $amount; ++ } + ++ public function deposit(int $amount): void ++ { ++ $this->balance += $amount; ++ } + ++    public function getBalance(): int ++ { ++ return $this->balance; ++ } +} + +$bankAccount = new BankAccount(); + +// Buy shoes... +- $bankAccount->balance -= 100; ++ $bankAccount->withdraw($shoesPrice); + +// Get balance ++ $balance = $bankAccount->getBalance(); +``` +
+ **[⬆ back to top](#table-of-contents)** ### Make objects have private/protected members @@ -1801,6 +2033,35 @@ $employee = new Employee('John Doe'); echo 'Employee name: ' . $employee->getName(); ``` +
+ +Refactor: + + +```diff +class Employee +{ +- public $name; ++ private $name; + + public function __construct(string $name) + { + $this->name = $name; + } + ++ public function getName(): string ++ { ++ return $this->name; ++ } +} + +$employee = new Employee('John Doe'); +// Employee name: John Doe +- echo 'Employee name: ' . $employee->name; ++ echo 'Employee name: ' . $employee->getName(); +``` +
+ **[⬆ back to top](#table-of-contents)** ## Classes @@ -1904,6 +2165,71 @@ class Employee } ``` +
+ +Refactor: + + +```diff ++ class EmployeeTaxData ++ { ++ private $ssn; + ++ private $salary; + ++ public function __construct(string $ssn, string $salary) ++ { ++ $this->ssn = $ssn; ++ $this->salary = $salary; ++ } + ++ // ... ++ } + +class Employee +{ + private $name; + + private $email; + ++ private $taxData; + + public function __construct(string $name, string $email) + { + $this->name = $name; + $this->email = $email; + } + ++ public function setTaxData(EmployeeTaxData $taxData): void ++ { ++ $this->taxData = $taxData; ++ } + ++ // ... +} + +// Bad because Employees "have" tax data. +// EmployeeTaxData is not a type of Employee + +- class EmployeeTaxData extends Employee +- { +- private $ssn; + +- private $salary; + +- public function __construct(string $name, string $email, string $ssn, string $salary) +- { +- parent::__construct($name, $email); + +- $this->ssn = $ssn; +- $this->salary = $salary; +- } + +- // ... +-} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Avoid fluent interfaces @@ -2011,6 +2337,66 @@ $car->setModel('F-150'); $car->dump(); ``` +
+ +Refactor: + + +```diff +class Car +{ + private $make = 'Honda'; + + private $model = 'Accord'; + + private $color = 'white'; + +- public function setMake(string $make): self ++ public function setMake(string $make): void + { + $this->make = $make; + +- // NOTE: Returning this for chaining +- return $this; + } + +- public function setModel(string $model): self ++ public function setModel(string $model): void + { + $this->model = $model; + +- // NOTE: Returning this for chaining +- return $this; + } + +- public function setColor(string $color): self ++ public function setColor(string $color): void + { + $this->color = $color; + +- // NOTE: Returning this for chaining +- return $this; + } + + public function dump(): void + { + var_dump($this->make, $this->model, $this->color); + } +} + +- $car = (new Car()) +- ->setColor('pink') +- ->setMake('Ford') +- ->setModel('F-150') +- ->dump(); ++ $car = new Car(); ++ $car->setColor('pink'); ++ $car->setMake('Ford'); ++ $car->setModel('F-150'); ++ $car->dump(); +``` +
+ **[⬆ back to top](#table-of-contents)** ### Prefer final classes @@ -2076,6 +2462,41 @@ final class Car implements Vehicle } ``` +
+ +Refactor: + + +```diff ++ interface Vehicle ++ { ++ /** ++ * @return string The color of the vehicle ++ */ ++ public function getColor(); ++ } + +- final class Car ++ final class Car implements Vehicle +{ + private $color; + + public function __construct($color) + { + $this->color = $color; + } + +- /** +- * @return string The color of the vehicle +- */ + public function getColor() + { + return $this->color; + } +} +``` +
+ **[⬆ back to top](#table-of-contents)** ## SOLID @@ -2166,6 +2587,55 @@ class UserSettings } ``` +
+ +Refactor: + + +```diff ++ class UserAuth ++ { ++ private $user; + ++ public function __construct(User $user) ++ { ++ $this->user = $user; ++ } + ++ public function verifyCredentials(): bool ++ { ++ // ... ++ } ++ } + +class UserSettings +{ + private $user; + ++ private $auth; + + public function __construct(User $user) + { + $this->user = $user; ++ $this->auth = new UserAuth($user); + } + + public function changeSettings(array $settings): void + { +- if ($this->verifyCredentials()) { ++ if ($this->auth->verifyCredentials()) { + // ... + } + } + +- private function verifyCredentials(): bool +- { +- // ... +- } +} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Open/Closed Principle (OCP) @@ -2280,6 +2750,91 @@ class HttpRequester } ``` +
+ +Refactor: + + +```diff +- abstract class Adapter ++ interface Adapter +{ +- protected $name; + +- public function getName(): string +- { +- return $this->name; +- } + ++ public function request(string $url): Promise; +} + +- class AjaxAdapter extends Adapter ++ class AjaxAdapter implements Adapter +{ +- public function __construct() +- { +- parent::__construct(); + +- $this->name = 'ajaxAdapter'; +- } + ++ public function request(string $url): Promise ++ { ++ // request and return promise ++ } +} + +- class NodeAdapter extends Adapter ++ class NodeAdapter implements Adapter +{ +- public function __construct() +- { +- parent::__construct(); + +- $this->name = 'nodeAdapter'; +- } + ++ public function request(string $url): Promise ++ { ++ // request and return promise ++ } +} + +class HttpRequester +{ + private $adapter; + + public function __construct(Adapter $adapter) + { + $this->adapter = $adapter; + } + + public function fetch(string $url): Promise + { +- $adapterName = $this->adapter->getName(); + +- if ($adapterName === 'ajaxAdapter') { +- return $this->makeAjaxCall($url); +- } elseif ($adapterName === 'httpNodeAdapter') { +- return $this->makeHttpCall($url); +- } ++ return $this->adapter->request($url); + } + +- private function makeAjaxCall(string $url): Promise +- { +- // request and return promise +- } + +- private function makeHttpCall(string $url): Promise +- { +- // request and return promise +- } +} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Liskov Substitution Principle (LSP) @@ -2341,7 +2896,7 @@ function printArea(Rectangle $rectangle): void $rectangle->setHeight(5); // BAD: Will return 25 for Square. Should be 20. - echo sprintf('%s has area %d.', get_class($rectangle), $rectangle->getArea()) . PHP_EOL; + echo sprintf('%s has area %d.', get_class($rectangle), $rectangle->getArea()).PHP_EOL; } $rectangles = [new Rectangle(), new Square()]; @@ -2409,6 +2964,90 @@ foreach ($shapes as $shape) { } ``` +
+ +Refactor: + + +```diff ++ interface Shape ++ { ++ public function getArea(): int; ++ } + +- class Rectangle ++ class Rectangle implements Shape +{ +- protected $width = 0; ++ private $width = 0; +- protected $height = 0; ++ private $height = 0; + +- public function setWidth(int $width): void ++ public function __construct(int $width, int $height) + { + $this->width = $width; +- } +- +- public function setHeight(int $height): void +- { + $this->height = $height; + } + + public function getArea(): int + { + return $this->width * $this->height; + } +} + +- class Square extends Rectangle ++ class Square implements Shape +{ ++ private $length = 0; + +- public function setWidth(int $width): void +- { +- $this->width = $this->height = $width; +- } + +- public function setHeight(int $height): void +- { +- $this->width = $this->height = $height; +- } + ++ public function __construct(int $length) ++ { ++ $this->length = $length; ++ } + ++ public function getArea(): int ++ { ++        return $this->length ** 2; ++    } +} + +- function printArea(Rectangle $rectangle): void ++ function printArea(Shape $shape): void +{ +- $rectangle->setWidth(4); +- $rectangle->setHeight(5); + +- // BAD: Will return 25 for Square. Should be 20. +- echo sprintf('%s has area %d.', get_class($rectangle), $rectangle->getArea()).PHP_EOL; ++ echo sprintf('%s has area %d.', get_class($shape), $shape->getArea()).PHP_EOL; +} + +- $rectangles = [new Rectangle(), new Square()]; ++ $shapes = [new Rectangle(4, 5), new Square(5)]; + +- foreach ($rectangles as $rectangle) { ++ foreach ($shapes as $shape) { +- printArea($rectangle); ++ printArea($shape); +} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Interface Segregation Principle (ISP) @@ -2500,6 +3139,57 @@ class RobotEmployee implements Workable } ``` +
+ +Refactor: + + +```diff +- interface Employee ++ interface Workable +{ + public function work(): void; ++ } ++ ++ interface Feedable ++ { + public function eat(): void; +} + ++ interface Employee extends Feedable, Workable ++ { ++ } + +class HumanEmployee implements Employee +{ + public function work(): void + { + // ....working + } + + public function eat(): void + { + // ...... eating in lunch break + } +} + ++ // robot can only work +- class RobotEmployee implements Employee ++ class RobotEmployee implements Workable +{ + public function work(): void + { + //.... working much more + } +- +- public function eat(): void +- { +- //.... robot can't eat, but it must implement this method +- } +} +``` +
+ **[⬆ back to top](#table-of-contents)** ### Dependency Inversion Principle (DIP) @@ -2592,6 +3282,48 @@ class Manager } ``` +
+ +Refactor: + + +```diff +- class Employee ++ interface Employee +{ +- public function work(): void ++ public function work(): void; +- { +- // ....working +- } +} + +- class Robot extends Employee ++ class Robot implements Employee +{ + public function work(): void + { + //.... working much more + } +} + +class Manager +{ + private $employee; + + public function __construct(Employee $employee) + { + $this->employee = $employee; + } + + public function manage(): void + { + $this->employee->work(); + } +} +``` +
+ **[⬆ back to top](#table-of-contents)** ## Don’t repeat yourself (DRY) @@ -2676,6 +3408,53 @@ function showList(array $employees): void } ``` +
+ +Refactor: + + +```diff +- function showDeveloperList(array $developers): void +- { +- foreach ($developers as $developer) { +- $expectedSalary = $developer->calculateExpectedSalary(); +- $experience = $developer->getExperience(); +- $githubLink = $developer->getGithubLink(); +- $data = [$expectedSalary, $experience, $githubLink]; +- +- render($data); +- } +- } + +- function showManagerList(array $managers): void +- { +- foreach ($managers as $manager) { +- $expectedSalary = $manager->calculateExpectedSalary(); +- $experience = $manager->getExperience(); +- $githubLink = $manager->getGithubLink(); +- $data = [$expectedSalary, $experience, $githubLink]; +- +- render($data); +- } +- } + ++ function showList(array $employees): void ++ { ++ foreach ($employees as $employee) { +- $expectedSalary = $employee->calculateExpectedSalary(); +- $experience = $employee->getExperience(); +- $githubLink = $employee->getGithubLink(); +- $data = [$expectedSalary, $experience, $githubLink]; + +- render($data); ++ render([$employee->calculateExpectedSalary(), $employee->getExperience(), $employee->getGithubLink()]); ++ } ++ } + + +``` +
+ **[⬆ back to top](#table-of-contents)** ## Translations From ce26ab922f5bde6fb1404aa757bb41b7492de747 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 24 Nov 2022 03:24:19 +0700 Subject: [PATCH 17/18] delete: unnecessary refactor code --- README.md | 105 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 7bd0a5f7..a89c5e59 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,7 @@ $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT ```diff +- // What the heck is 448 for? - $result = $serializer->serialize($data, 448); + $json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE); ``` @@ -212,25 +213,30 @@ $user->access ^= User::ACCESS_CREATE; ```diff class User { +- // What the heck is 7 for? - public $access = 7; + public const ACCESS_READ = 1; -+ + + public const ACCESS_CREATE = 2; -+ + + public const ACCESS_UPDATE = 4; -+ + + public const ACCESS_DELETE = 8; -+ + ++ // User as default can read, create and update something + public $access = self::ACCESS_READ | self::ACCESS_CREATE | self::ACCESS_UPDATE; } +- // What the heck is 4 for? - if ($user->access & 4) { + if ($user->access & User::ACCESS_UPDATE) { - // ... + // do edit ... } +- // What's going on here? - $user->access ^= 2; ++ // Deny access rights to create something + $user->access ^= User::ACCESS_CREATE; ```
@@ -364,9 +370,9 @@ function isShopOpen(string $day): bool + if (empty($day)) { + return false; + } -+ + + $openingDays = ['friday', 'saturday', 'sunday']; -+ + + return in_array(strtolower($day), $openingDays, true); } ``` @@ -433,11 +439,11 @@ function fibonacci(int $n): int + if ($n === 0 || $n === 1) { + return $n; + } -+ + + if ($n >= 50) { + throw new Exception('Not supported'); + } -+ + + return fibonacci($n - 1) + fibonacci($n - 2); } ``` @@ -653,7 +659,7 @@ $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; - $name = $_POST['name']; - } else { - $name = 'nobody'; --} +- } + $name = $_GET['name'] ?? $_POST['name'] ?? 'nobody'; ```
@@ -816,56 +822,56 @@ class Questionnaire ```diff -+class Name -+{ ++ class Name ++ { + private $firstname; -+ + + private $lastname; -+ + + private $patronymic; -+ + + public function __construct(string $firstname, string $lastname, string $patronymic) + { + $this->firstname = $firstname; + $this->lastname = $lastname; + $this->patronymic = $patronymic; + } -+ + + // getters ... -+} -+ -+class City -+{ ++ } + ++ class City ++ { + private $region; -+ + + private $district; -+ + + private $city; -+ + + public function __construct(string $region, string $district, string $city) + { + $this->region = $region; + $this->district = $district; + $this->city = $city; + } -+ + + // getters ... -+} -+ -+class Contact -+{ ++ } + ++ class Contact ++ { + private $phone; -+ + + private $email; -+ + + public function __construct(string $phone, string $email) + { + $this->phone = $phone; + $this->email = $email; + } -+ + + // getters ... -+} ++ } + class Questionnaire { @@ -945,8 +951,9 @@ class Email } $message = new Email(...); -// +- // What is this? A handle for the message? Are we writing to a file now? - $message->handle(); ++ // Clear and obvious + $message->send(); ``` @@ -1209,7 +1216,7 @@ function createTempFile(string $name): void + { + touch($name); + } -+ + + function createTempFile(string $name): void + { + touch('./temp/' . $name); @@ -1279,16 +1286,20 @@ var_dump($newName); ```diff +- // Global variable referenced by following function. +- // If we had another function that used this name, now it'd be an array and it could break it. +- $name = 'Ryan McDermott'; + - function splitIntoFirstAndLastName(): void + function splitIntoFirstAndLastName(string $name): array { - global $name; -- + - $name = explode(' ', $name); + return explode(' ', $name); } -$name = 'Ryan McDermott'; ++ $name = 'Ryan McDermott'; - splitIntoFirstAndLastName(); + $newName = splitIntoFirstAndLastName($name); @@ -1361,19 +1372,19 @@ And now you must use the instance of `Configuration` in your application. + class Configuration + { + private $configuration = []; -+ + + public function __construct(array $configuration) + { + $this->configuration = $configuration; + } -+ + + public function get(string $key): ?string + { + // null coalescing operator + return $this->configuration[$key] ?? null; + } + } -+ + - function config(): array - { - return [ @@ -1467,7 +1478,7 @@ class DBConnection - if (self::$instance === null) { - self::$instance = new self(); - } -- + - return self::$instance; - } @@ -1975,7 +1986,7 @@ $bankAccount = new BankAccount(); - $bankAccount->balance -= 100; + $bankAccount->withdraw($shoesPrice); -// Get balance ++ // Get balance + $balance = $bankAccount->getBalance(); ``` @@ -2205,11 +2216,11 @@ class Employee + $this->taxData = $taxData; + } -+ // ... + // ... } -// Bad because Employees "have" tax data. -// EmployeeTaxData is not a type of Employee +- // Bad because Employees "have" tax data. +- // EmployeeTaxData is not a type of Employee - class EmployeeTaxData extends Employee - { @@ -2988,7 +2999,7 @@ foreach ($shapes as $shape) { { $this->width = $width; - } -- + - public function setHeight(int $height): void - { $this->height = $height; @@ -3181,7 +3192,7 @@ class HumanEmployee implements Employee { //.... working much more } -- + - public function eat(): void - { - //.... robot can't eat, but it must implement this method @@ -3421,7 +3432,7 @@ function showList(array $employees): void - $experience = $developer->getExperience(); - $githubLink = $developer->getGithubLink(); - $data = [$expectedSalary, $experience, $githubLink]; -- + - render($data); - } - } @@ -3433,7 +3444,7 @@ function showList(array $employees): void - $experience = $manager->getExperience(); - $githubLink = $manager->getGithubLink(); - $data = [$expectedSalary, $experience, $githubLink]; -- + - render($data); - } - } From 89c365038aab5186aa8fd1fff7d00fe18ada5a77 Mon Sep 17 00:00:00 2001 From: Retiago Drago <36787606+ranggakd@users.noreply.github.com> Date: Thu, 8 Dec 2022 16:19:30 +0700 Subject: [PATCH 18/18] chore: deleting whitespaces --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index a89c5e59..1e7e2560 100644 --- a/README.md +++ b/README.md @@ -3461,8 +3461,6 @@ function showList(array $employees): void + render([$employee->calculateExpectedSalary(), $employee->getExperience(), $employee->getGithubLink()]); + } + } - - ```