[Refactoring] Bad Smells in Code

There are no clear rules for when to refactor. However, there are characteristics of code that desperately need refactoring. Kent Beck defines this as smell(bad smell).

strange name

Code should be written simply and clearly. In particular, the name should make it clear what the code does just by looking at it. These efforts will soon make the code easier to understand.

If you can’t think of a good name, there may be a fundamental problem with the design. Organizing the names well can sometimes make the code concise.

duplicate code

If the same code structure is repeated, you can create a better program by combining it into one.

Refactoring	explanation
Extracting Functions	For simple duplicate code Call a method extracted from duplicate code
Slide a sentence	In cases where they are not exactly the same but are similar Collect similar parts in one place to make it easier to apply Extracting the function
Post method	Duplicate code between child classes is moved to the parent

long function

Refactoring	explanation
short function	The longer the function, the harder it is to understand The effect of indirect calls, making code easier to understand, share, and select
good name	Make the parts that need comments into functions The body of the function contains the code you wanted to explain with comments. The function name is named intent, not method of operation.

Long parameter list

The longer the parameters become, the more difficult it is to understand.

Refactoring	explanation
Converting parameters into query functions	Remove parameters whose values ​​can be obtained from other parameters
Passing the entire object	Pass to the original data structure
Create a parameter object	Binding parameters that are always passed together
Remove flag argument	Remove flags that control how functions operate.
Bundle multiple functions into a class	Defining common values ​​as fields of a class Useful when multiple functions commonly use the values ​​of specific parameters

global data

The problem is that global data can be touched anywhere in the code base, and there is no mechanism to find out who changed the value.

Refactoring	explanation
Encapsulating Variables	Wrap data in a function Easily find the part where data is modified Access control possible

variable data

Changing data often leads to unexpected results or annoying bugs.

Refactoring	explanation
Encapsulating Variables	Set the value to be changed only through a designated function Can monitor how the value is modified Easy to improve the code
Split Variables	Values ​​for different purposes are stored in independent variables for each purpose It is better to separate update logic from other code Use statement sliding and function splitting Separate code without side effects from code that updates something
Separate query and change functions	Prevent calling code with side effects
Removing the setter	Helpful in reducing the effective range of variables
Converting derived variables into query functions	Variable data whose value can be set elsewhere stinks
Bundle multiple functions into a class/transformation function	Variables with a wide effective range have a high risk of causing problems Limit the effective range of codes that update variables
Converting References to Values	In the case of variables that contain data in internal fields, such as structures It is better to replace the entire structure rather than modify the internal fields directly

tangled change

The structure of software should be organized in a way that is easy to change. Otherwise, Tangled Change or Shotgun Surgery will appear.

Convoluted changes occur when SRP (Single Responsibility Principle) is not satisfied. In other words, it occurs when one module is changed a lot for different reasons.

Since business logic and database integration occur in different contexts, they should be configured as independent modules. However, it is not easy to divide these boundaries in the early stages of development, and these boundaries are constantly changing during the development process.

Refactoring	explanation
step splitting	In the case of sequential contexts such as fetching and processing data from business logic Separate the steps by having the data required for the next context be delivered in a specific data structure
Moving functions	When functions from different contexts are frequently called throughout the entire processing process Create appropriate modules for each context and collect related functions
Extracting Functions	When moving a function, if there is a function involved in multiple contexts
Extracting classes	If the module is a class, separate it by context by extracting the class

shotgun surgery

Shotgun surgery is similar to, but also the exact opposite of, tangled change.

This smell arises when there are many classes that require minor modifications every time the code is changed. If the parts to be changed are spread throughout the code, it is difficult to find them and it is easy to miss the places that really need to be changed.

Refactoring	explanation
Moving functions / Moving fields	Bundle the objects that change into one module
Bundle multiple functions into a class	When there are many functions that handle similar data
Bundle multiple functions into a conversion function	For functions that transform or augment data structures
step splitting	When passing the output results of functions grouped like this to the next stage of logic
Inlining functions / Inlining classes	A good way to deal with shotgun surgery caused by poorly separated logic Blended methods or classes, but improved with later extract refactoring

functional favoritism

A smell that arises when a function interacts more with other modules than with the functions or data of the module it belongs to.

Refactoring	explanation
Moving functions	Close related functions and data
Extracting Functions	If you prefer a function to only part of a function Extract that part and an independent function Move to the desired module with Move function

If it’s not clear where to move, move to the module that contains the most data.

bunch of data

A bundle of data that always moves together deserves a separate home.

Refactoring	explanation
Extracting classes	Bundle of field-type data into one object
Creating a parameter object / Passing the entire object	For chunks of data in method signatures Apply this refactoring to reduce number of parameters

Taking a class gives you the opportunity to spread a good scent.

basic obsession

There are many people who are obsessed with the basic types provided by programming languages ​​and are reluctant to directly define basic types appropriate for a given problem.

• Converting primitives to objects
  • It looks like it’s just wrapping a basic data type.
  • Special actions can be added if necessary
• When code expressed as a basic type is used as a type code to control conditional operation
  • Convert type code to subclass
  • Converting conditional logic to polymorphism
  • Apply the above refactoring sequentially.

Repeated switch statement

It is better to eliminate switch statements as much as possible by changing conditional logic to polymorphism. This is because every time you add a conditional clause, you need to find all other switch statements and modify them as well.

repetition

Nowadays, many languages ​​​​support first-class function. Let’s apply Convert Loops to Pipelines to remove loops that are not appropriate for the times. Using pipeline operations such as filters or maps makes it easy to understand how each element is processed in the code.

insincere elements

In fact, in the case of poor modules, such as classes with only one method, removal is performed. Processed as inlining a function or inlining a class. If inheritance is used, use combining layers.

speculative generalization

It comes from code that has all kinds of hooking points and special case handling logic that are not needed right now.

• Abstract class that does very little
  • Merge Layers
• Useless delegation code
  • Deleted by inlining a function or inlining a class
• Parameters not used in the body
  • Delete by replacing function declaration
• A function or class that is used nowhere else than in test code.
  • Removing dead code

temporary field

Sometimes there are classes that have fields whose values ​​are set only under certain circumstances. In these cases, users are left scratching their heads trying to figure out why a seemingly unused field exists.

1. If you find scattered fields, collect them with Extract classes.
2. Put the code related to temporary fields into the class created earlier using Move Function.
3. The conditional logic that operates after checking whether temporary fields are valid is Add a special case, which creates an alternative class for when the fields are invalid and removes them.

References

• Martin Fowler, 『Refactoring 2nd Edition』, Hanbit Media