Refactor sets: Difference between revisions
imported>Unknown user No edit summary |
imported>Unknown user No edit summary |
||
| Line 1: | Line 1: | ||
<span id="general"></span> | |||
== General == | |||
* '''intention vs implementation'''. Always separate the intention of the fragment of code from its implementation | |||
* '''naming'''. Coming up to good names is hard. A good way is creating a comment describing the purpose of the variable / function / class and create the name from that comment. | |||
* '''remove dead code''': it makes the code more difficult to understand | |||
* '''re-refactor'''. During a refactor, we can notice that we can reorient the behaviour or the way to code the program. | |||
== functions == | |||
* '''extract function''': extract a fragment of code into a function based on length and reusability. If you need to spend time figuring out "what" a fragment of code does, you need a function named "what" with that fragment of code. | |||
* '''inline function''': if the intention of a function is clear, code is self-explanatory and really short, get rid of the function and mode fragment of code to the original function call. | |||
* '''move statements into function''': if a statement is more convenient to be inside an existing function, move it inside it | |||
* '''move statements into callers''': if a statement is more convenient to be outside an existing function, move it outside the function. One trigger for this is when common behaviour used in several places needs to vary in some of its callers. | |||
* '''rename function''': See #General#naming | |||
* '''combine functions into class''': using a class for similar context functions makes more explicit the common context they share. | |||
* '''replace function with command''': similar to "combine functions into class" | |||
* '''replace command with function''': if the intention is not too complex, then a command (class) is more trouble that its worth. | |||
* '''multiple parameters to object''': when the same group of variables are sent to different functions, we can group them into objects. | |||
== variables == | |||
* '''extract variable''': break long expressions with multiple intentions in separate expressions with multiple variables. | |||
* '''inline variables''': variables that do not have lots of complexity for its calculation, can be deleted and its calculation moved to where the variable was used. | |||
* '''rename variable''': See #General#naming | |||
* '''encapsulate variable''': reduce the scope where the variable is used by only modifying it into common functions. | |||
* '''split variable''': a variable with more than one responsibility should be replaced by well-named multiple variables. | |||
== encapsulation == | |||
<ul> | |||
<li><p>'''encapsulate record''': Data is often treated as a simple variable or array. If the data is complex, it is convenient to create an object from it (DTO, VO or Entity)</p></li> | |||
<li><p>'''replace Temp with Query''': similar to encapsulate record / variable but for temporary data</p></li> | |||
<li><p>'''replace derived variable with query''': minimize the scope / places where data is changed, try to organize them as easy as possible.</p></li> | |||
<li><p>'''extract class''': similar than previous extracts but creating classes for using in another classes.</p></li> | |||
<li><p>'''inline class''': when extract class has been used but it's really not needed.</p></li> | |||
<li><p>'''hide delegate''': indirect use of extract class. Parent class is called but it uses child class indirectly.</p> | |||
<syntaxhighlight>$phoneNumber = $person->getPhoneNumber(); | |||
to | |||
class Person { getPhoneNumber() { return $this->phone->getNumber(); } }</syntaxhighlight></li> | |||
<li><p>'''remove middle man''': inverse of hide delegate. When delegation method is overused, you can simply remove it an access the subobject directly. This can generate an issue with properties visibility.</p> | |||
<syntaxhighlight>$phoneNumber = $person->phone->number;</syntaxhighlight></li> | |||
<li><p>'''replace primitive with object''': when we have a simple operation with object information, delegate the implementation of this intention to the object.</p> | |||
<syntaxhighlight>$orderIsPrioritary = in_array($order->priority, ['high', 'critital']); | |||
to | |||
$orderIsPrioritary = $order->isPrioritary();</syntaxhighlight></li> | |||
<li><p>'''change reference to value''': Instead of storing in a field the reference to another class' field, we store the whole nested class.</p> | |||
<syntaxhighlight>applyDiscount($discount) { | |||
$this->price->amount -= $discount; | |||
to | |||
$this->price = New Money($this->price->amount - $discount);</syntaxhighlight></li> | |||
<li><p>'''change value to reference''': if we have a static value of data, if we have multiple instances of the same data, we don't have it centralized. Instead, we need a reference to the data instead of the value itself.</p> | |||
<syntaxhighlight>$customer = new Customer($customerData); | |||
to | |||
$customer = $customerRepository->get($customerData->id); | |||
</syntaxhighlight></li></ul> | |||
<span id="moving-features"></span> | |||
== Moving features == | |||
<ul> | |||
<li><p>'''move method''': methods of classes need to belong to the proper class that has the whole context of the intention of the method.</p> | |||
<syntaxhighlight>Class Order() { getType() { return uppercase($this->type); } } | |||
to | |||
Class Order() { getType() { return $this->type->get(); } } | |||
Class OrderType() { get() { return uppsercase($this->value); } } | |||
</syntaxhighlight></li> | |||
<li><p>'''move field''': same as for moving methods apply to fields of classes. Fields have to be in the class where makes more sense for that field intention.</p></li> | |||
<li><p>'''replace inline code with function call''': when we have a code that is already on an existing function / method, replace it.</p></li> | |||
<li><p>'''split loop''': if a loop has two separate intentions, when you have to modify the loop you have to understand both intentions. Splitting the two intentions in different loops make it easier to use and modify.</p></li> | |||
<li><p>'''replace loop with pipeline''': instead of using loops use language pipelines when it becomes easier to understand.</p></li></ul> | |||
<span id="structure"></span> | |||
== Structure == | |||
* '''split phase''': if a long code implementation has different intentions, we have to arrange them so each intention is code consecutively in the same phase. You can then #General#re-refactor | |||
* '''slide statements''': similiar to slide phases, but without extracting functions. | |||
<span id="conditional-logic"></span> | |||
== Conditional logic == | |||
<ul> | |||
<li><p>'''decompose conditional''': instead of having multiple conditions, replace them with a function which name is the "what" of the condition</p> | |||
<syntaxhighlight>if($date->isBefore(DAY_SUMMER_STARTS) && $date->isAfter(DATE_SUMMER_ENDS)) $roomTemperature = $baseTemperature - 5; | |||
else $roomTemperature = $baseTemperature + 5; | |||
to | |||
if(summer()) $roomTemperature = summerTemperature(); | |||
else $roomTemperature = coldTemperature();</syntaxhighlight></li> | |||
<li><p>'''consolidate conditional expression''': merge multiple checks resulting in the same action</p> | |||
<syntaxhighlight>if($temperature == '40') return 'summer'; | |||
if($date == '2023-08') return 'summer'; | |||
to | |||
if(summer()) return 'summer'; | |||
summer() { return $temperature ## '40' || $date '2023-08'; }</syntaxhighlight></li> | |||
<li><p>'''replace nested conditional with guard clauses''': in conditionals both if/else have the same importance. If one of them is unsual, we have to reflect it. We can make it with guard clauses. The key point is emphazise that there could be abnormal conditions during the calculation.</p> | |||
<syntaxhighlight>getPayAmount($status) { | |||
if($status == 'isDead') { | |||
$result = deadAmount(); | |||
} | |||
else { | |||
if($status == 'isSeparated') { | |||
$result = separatedAmount(); | |||
} else { | |||
if($status == 'isRetired') { | |||
$result = retiredmount(); | |||
} else { | |||
return normalPayment(); | |||
} | |||
} | |||
} | |||
} | |||
to | |||
getPayAmount($status) { | |||
if($status == 'isDead') return deadAmount(); | |||
if($status == 'isSeparated') return separatedAmount(); | |||
if($status == 'isRetired') return retiredmount(); | |||
return normalPayment(); | |||
}</syntaxhighlight></li> | |||
<li><p>'''replace conditional with polymorphism''': instead of calculating into conditional a value, use polymorphism to delegate the conditional responsibilities to the classes.</p> | |||
<syntaxhighlight>getClothes() { | |||
switch($season) { | |||
case 'spring': return 't-shirt'; | |||
case 'summer': return 'swimsuit'; | |||
case 'autumn': return 'jersey'; | |||
case 'winter': return 'coat'; | |||
default: return 'jeans'; | |||
} | |||
} | |||
to | |||
getClothes() { getSeason().clothes(); } | |||
getSeason() { | |||
switch($season) { | |||
case 'spring': return SpringSeason(); | |||
case 'summer': return SummerSeason(); | |||
case 'autumn': return AutumnSeason(); | |||
case 'winter': return WinterSeason(); | |||
default: return AllSeason(); | |||
} | |||
}</syntaxhighlight></li> | |||
<li><p>'''introduce special case''': Aka Null Object Pattern. A common case of duplicated code is when many users of a data structure check a specific value, and then most of them do the same thing. That action needs to be in a single place.</p></li> | |||
<li><p>'''introduce assertion''': An assertion is a conditional statement that is assumed to be always true. Failure of an assertion indicates a programmer error. Assertions should be written so that the program functions equally correctly if they are all removed. They tell the reader something about the assumed state of the program at this point of execution. They are used to check things that need to be true.</p></li></ul> | |||
<span id="refactoring-apis"></span> | |||
== Refactoring APIs == | |||
<ul> | |||
<li><p>'''separate query from modifier''': It is helpful having a difference between functions with side effects. This is the commonly known Command Query Segregation.</p></li> | |||
<li><p>'''parameterize function''': If two functions have similar logic, merge them into a single function with an additional parameter por the different cases. This increases the usefulness of the function. This parameter must not be a flag por conditional expressions (see remove flag argument)</p> | |||
<syntaxhighlight>function increaseFivePercent($value) { return $value * 1.05; } | |||
function increaseTenPercent($value) { return $value * 1.1; } | |||
to | |||
function increasePercentage($value, $factor) { return $value * $factor; }</syntaxhighlight></li> | |||
<li><p>'''remove flag argument''': Aka Replace Parameter with Explicit Methods. They complicate the process of understanding what function calls are available and how to call them.</p> | |||
<syntaxhighlight>function setDimension($name, $value) { | |||
if($name == 'height') $this->height = $value; | |||
if($name == 'width') $this->width = $value; | |||
return; | |||
} | |||
to | |||
function setHeight($value) {} | |||
function setWidth($value) {}</syntaxhighlight></li> | |||
<li><p>'''preserve whole object''': fragment of code that derives a couple of values from an object and then passes these values into a function (similar to multiple parameters to object)</p></li> | |||
<li><p>'''replace parameter with query''': if a function call passes in a value that the function can just as easily determine fo itself, that's a form of unnecessarily complicate the use of that function. Shift the responsibility of obtaining that value to the function.</p></li> | |||
<li><p>'''replace query with parameter''': this query (variable) inside a function might be a reference to a global variable or an element intended to be moved away. Parametrizing gives the responsibility to the client and allows us to have different sources for that function parameter.</p></li> | |||
<li><p>'''remove setting method''': Having a setting method indicates that field may be changed. For simple Data Transfer Objects, attributes must only be set in the constructor.</p></li> | |||
<li><p>'''replace constructor with Factory function''': use Factory Pattern as seen in "replace conditional with polymorphism".</p></li></ul> | |||
<span id="dealing-with-inheritance"></span> | |||
== Dealing with inheritance == | |||
<ul> | |||
<li><p>'''pull up method''': it must be applied when two methods have the same exact body. When they are very similar, "parameterize function" can be applied to them. This method will be moved to their parent class.</p></li> | |||
<li><p>'''push down method''': if a method is only relevant to some subclass, move it from the parent to the subclasses.</p></li> | |||
<li><p>'''pull up field''': if two fields in different classes are used in the same way, they can be moved to their parent class.</p></li> | |||
<li><p>'''push down field''': if a field is only relevant to some subclasses, move it from the parent to the subclasses.</p></li> | |||
<li><p>'''pull up constructor body''': if all constructors of the implementations of a class have a common code, this code can be moved to the constructor body and all implementations constructors may call also their parent body.</p> | |||
<syntaxhighlight>class Car extends Vehicle { | |||
constructor(surface) { | |||
$this->surface = 'land'; | |||
$this->numberOfDoors = 3; | |||
} | |||
} | |||
class Wetbike extends Vehicle { | |||
constructor(surface) { | |||
$this->surface = 'water'; | |||
} | |||
} | |||
to | |||
class Vehicle { constructor($surface) { $this->surface = $surface; } | |||
class Car { constructor($surface) { parent::construct($surface); }</syntaxhighlight></li> | |||
<li><p>'''replace type code with subclasses''': instead of having an attribute for that class, use polymorphism to create a set of subclasses.</p></li> | |||
<li><p>'''remove subclass''': Subclasses can lose their value as their code is moved or removed. It also might have been created with the intention of adding more features. If a subclass does too little incurs a cost in understanding that is not worthwhile. When that happens, replace the subclass with a field on the superclass.</p></li> | |||
<li><p>'''collapse hierarchy''': If a class and its parent are no longer different enough, merge them.</p></li></ul> | |||
<!-- From bodyContent 1543053.mraw --> | <!-- From bodyContent 1543053.mraw --> | ||
Revision as of 11:39, 19 April 2025
General
- intention vs implementation. Always separate the intention of the fragment of code from its implementation
- naming. Coming up to good names is hard. A good way is creating a comment describing the purpose of the variable / function / class and create the name from that comment.
- remove dead code: it makes the code more difficult to understand
- re-refactor. During a refactor, we can notice that we can reorient the behaviour or the way to code the program.
functions
- extract function: extract a fragment of code into a function based on length and reusability. If you need to spend time figuring out "what" a fragment of code does, you need a function named "what" with that fragment of code.
- inline function: if the intention of a function is clear, code is self-explanatory and really short, get rid of the function and mode fragment of code to the original function call.
- move statements into function: if a statement is more convenient to be inside an existing function, move it inside it
- move statements into callers: if a statement is more convenient to be outside an existing function, move it outside the function. One trigger for this is when common behaviour used in several places needs to vary in some of its callers.
- rename function: See #General#naming
- combine functions into class: using a class for similar context functions makes more explicit the common context they share.
- replace function with command: similar to "combine functions into class"
- replace command with function: if the intention is not too complex, then a command (class) is more trouble that its worth.
- multiple parameters to object: when the same group of variables are sent to different functions, we can group them into objects.
variables
- extract variable: break long expressions with multiple intentions in separate expressions with multiple variables.
- inline variables: variables that do not have lots of complexity for its calculation, can be deleted and its calculation moved to where the variable was used.
- rename variable: See #General#naming
- encapsulate variable: reduce the scope where the variable is used by only modifying it into common functions.
- split variable: a variable with more than one responsibility should be replaced by well-named multiple variables.
encapsulation
encapsulate record: Data is often treated as a simple variable or array. If the data is complex, it is convenient to create an object from it (DTO, VO or Entity)
replace Temp with Query: similar to encapsulate record / variable but for temporary data
replace derived variable with query: minimize the scope / places where data is changed, try to organize them as easy as possible.
extract class: similar than previous extracts but creating classes for using in another classes.
inline class: when extract class has been used but it's really not needed.
hide delegate: indirect use of extract class. Parent class is called but it uses child class indirectly.
$phoneNumber = $person->getPhoneNumber(); to class Person { getPhoneNumber() { return $this->phone->getNumber(); } }remove middle man: inverse of hide delegate. When delegation method is overused, you can simply remove it an access the subobject directly. This can generate an issue with properties visibility.
$phoneNumber = $person->phone->number;
replace primitive with object: when we have a simple operation with object information, delegate the implementation of this intention to the object.
$orderIsPrioritary = in_array($order->priority, ['high', 'critital']); to $orderIsPrioritary = $order->isPrioritary();
change reference to value: Instead of storing in a field the reference to another class' field, we store the whole nested class.
applyDiscount($discount) { $this->price->amount -= $discount; to $this->price = New Money($this->price->amount - $discount);change value to reference: if we have a static value of data, if we have multiple instances of the same data, we don't have it centralized. Instead, we need a reference to the data instead of the value itself.
$customer = new Customer($customerData); to $customer = $customerRepository->get($customerData->id);
Moving features
move method: methods of classes need to belong to the proper class that has the whole context of the intention of the method.
Class Order() { getType() { return uppercase($this->type); } } to Class Order() { getType() { return $this->type->get(); } } Class OrderType() { get() { return uppsercase($this->value); } }move field: same as for moving methods apply to fields of classes. Fields have to be in the class where makes more sense for that field intention.
replace inline code with function call: when we have a code that is already on an existing function / method, replace it.
split loop: if a loop has two separate intentions, when you have to modify the loop you have to understand both intentions. Splitting the two intentions in different loops make it easier to use and modify.
replace loop with pipeline: instead of using loops use language pipelines when it becomes easier to understand.
Structure
- split phase: if a long code implementation has different intentions, we have to arrange them so each intention is code consecutively in the same phase. You can then #General#re-refactor
- slide statements: similiar to slide phases, but without extracting functions.
Conditional logic
decompose conditional: instead of having multiple conditions, replace them with a function which name is the "what" of the condition
if($date->isBefore(DAY_SUMMER_STARTS) && $date->isAfter(DATE_SUMMER_ENDS)) $roomTemperature = $baseTemperature - 5; else $roomTemperature = $baseTemperature + 5; to if(summer()) $roomTemperature = summerTemperature(); else $roomTemperature = coldTemperature();
consolidate conditional expression: merge multiple checks resulting in the same action
if($temperature == '40') return 'summer'; if($date == '2023-08') return 'summer'; to if(summer()) return 'summer'; summer() { return $temperature ## '40' || $date '2023-08'; }replace nested conditional with guard clauses: in conditionals both if/else have the same importance. If one of them is unsual, we have to reflect it. We can make it with guard clauses. The key point is emphazise that there could be abnormal conditions during the calculation.
getPayAmount($status) { if($status == 'isDead') { $result = deadAmount(); } else { if($status == 'isSeparated') { $result = separatedAmount(); } else { if($status == 'isRetired') { $result = retiredmount(); } else { return normalPayment(); } } } } to getPayAmount($status) { if($status == 'isDead') return deadAmount(); if($status == 'isSeparated') return separatedAmount(); if($status == 'isRetired') return retiredmount(); return normalPayment(); }replace conditional with polymorphism: instead of calculating into conditional a value, use polymorphism to delegate the conditional responsibilities to the classes.
getClothes() { switch($season) { case 'spring': return 't-shirt'; case 'summer': return 'swimsuit'; case 'autumn': return 'jersey'; case 'winter': return 'coat'; default: return 'jeans'; } } to getClothes() { getSeason().clothes(); } getSeason() { switch($season) { case 'spring': return SpringSeason(); case 'summer': return SummerSeason(); case 'autumn': return AutumnSeason(); case 'winter': return WinterSeason(); default: return AllSeason(); } }introduce special case: Aka Null Object Pattern. A common case of duplicated code is when many users of a data structure check a specific value, and then most of them do the same thing. That action needs to be in a single place.
introduce assertion: An assertion is a conditional statement that is assumed to be always true. Failure of an assertion indicates a programmer error. Assertions should be written so that the program functions equally correctly if they are all removed. They tell the reader something about the assumed state of the program at this point of execution. They are used to check things that need to be true.
Refactoring APIs
separate query from modifier: It is helpful having a difference between functions with side effects. This is the commonly known Command Query Segregation.
parameterize function: If two functions have similar logic, merge them into a single function with an additional parameter por the different cases. This increases the usefulness of the function. This parameter must not be a flag por conditional expressions (see remove flag argument)
function increaseFivePercent($value) { return $value * 1.05; } function increaseTenPercent($value) { return $value * 1.1; } to function increasePercentage($value, $factor) { return $value * $factor; }remove flag argument: Aka Replace Parameter with Explicit Methods. They complicate the process of understanding what function calls are available and how to call them.
function setDimension($name, $value) { if($name == 'height') $this->height = $value; if($name == 'width') $this->width = $value; return; } to function setHeight($value) {} function setWidth($value) {}preserve whole object: fragment of code that derives a couple of values from an object and then passes these values into a function (similar to multiple parameters to object)
replace parameter with query: if a function call passes in a value that the function can just as easily determine fo itself, that's a form of unnecessarily complicate the use of that function. Shift the responsibility of obtaining that value to the function.
replace query with parameter: this query (variable) inside a function might be a reference to a global variable or an element intended to be moved away. Parametrizing gives the responsibility to the client and allows us to have different sources for that function parameter.
remove setting method: Having a setting method indicates that field may be changed. For simple Data Transfer Objects, attributes must only be set in the constructor.
replace constructor with Factory function: use Factory Pattern as seen in "replace conditional with polymorphism".
Dealing with inheritance
pull up method: it must be applied when two methods have the same exact body. When they are very similar, "parameterize function" can be applied to them. This method will be moved to their parent class.
push down method: if a method is only relevant to some subclass, move it from the parent to the subclasses.
pull up field: if two fields in different classes are used in the same way, they can be moved to their parent class.
push down field: if a field is only relevant to some subclasses, move it from the parent to the subclasses.
pull up constructor body: if all constructors of the implementations of a class have a common code, this code can be moved to the constructor body and all implementations constructors may call also their parent body.
class Car extends Vehicle { constructor(surface) { $this->surface = 'land'; $this->numberOfDoors = 3; } } class Wetbike extends Vehicle { constructor(surface) { $this->surface = 'water'; } } to class Vehicle { constructor($surface) { $this->surface = $surface; } class Car { constructor($surface) { parent::construct($surface); }replace type code with subclasses: instead of having an attribute for that class, use polymorphism to create a set of subclasses.
remove subclass: Subclasses can lose their value as their code is moved or removed. It also might have been created with the intention of adding more features. If a subclass does too little incurs a cost in understanding that is not worthwhile. When that happens, replace the subclass with a field on the superclass.
collapse hierarchy: If a class and its parent are no longer different enough, merge them.