Do not write duplicate code
Do not copy code
Write reusable and general code and call the existing code
If you copy the code, you need to fix the BUG in multiple places.
give an example
A system for managing bank accounts, which represents the flow process of money between different accounts through Transfer objects.
CheckingAccount refers to the checking account provided by the bank, as follows
public class CheckingAccount
{
private int transferLimit = 100;
public Transfer MakeTransfer(string counterAccount, Money amount)
{
//1 check withdrawal limit
if (amoiunt.GreaterThan(this.transferLimit))
{
throw new BusinessException("Limit exceeded!");
}
//2 assume that the result is a 9-digit bank account number, which is verified by 11 test
int sum = 0;
for (int i = 0; i < counterAccount.Length; i++)
{
sum += (9 - i) * (int)char.GetNumericValue(counterAccount[i]);
}
if (sum % 11 == 0)
{
//3 find the opposite account and create a transfer object
CheckingAccount acct = Accounts.FindAcctByNumber(counterAccount);
Transfer result = new Transfer(this, acct, amount);
return result;
}
else
{
throw new BusinessException("Invalid account number!");
}
}
}According to the account number of the opposite party, the MakeTransfer method will create a Transfer object. First check whether the Transfer amount exceeds the specified limit, then check whether the account number of the opposite party is valid, then obtain the object representing the account, and then create a Transfer object.
If a bank adds an account type, such as a savings account, it has no transfer limit, but there is another restriction: money can only be transferred to a designated checking account. To do this, you need to create a new class. Here is to copy CheckingAccount, rename it and make some adjustments
public class SavingAccount
{
CheckingAccount registeredCounterAccount;
public Transfer MakeTransfer(string counterAccount, Money amount)
{
//1 assume that the result is a 9-digit bank account number, which is verified by 11 test
int sum = 0;
for (int i = 0; i < counterAccount.Length; i++)
{
sum += (9 - i) * (int)char.GetNumericValue(counterAccount[i]);
}
if (sum % 11 == 0)
{
//2 find the opposite account and create a transfer object
CheckingAccount acct = Accounts.FindAcctByNumber(counterAccount);
Transfer result = new Transfer(this, acct, amount);
//3 check whether the withdrawing party is a registered opposite account
if (result.getCounterAccount().Equals(this.registeredCounterAccount))
{
return result;
}
else
{
throw new BusinessException("Counter-account not registered!");
}
}
else
{
throw new BusinessException("Invalid account number!");
}
}
}Assuming that there is a BUG in the place where we verify the bank account number, we need to modify the above two classes, which not only increases additional work, but also makes the maintenance inefficient.
The definition of duplicate code is that a piece of code with at least 6 lines of the same code (excluding blank lines and comments), also known as class 1 clone, is independent of the location of occurrence.
Two pieces of code that are syntactically identical are called class 2 clones, but differ in spaces, comments, identifiers, names, and literals. The principle applies to class 1 cloning.
According to SIG's experience, this 6-line code limit just reaches a balance point to avoid identifying too many or too few duplicate codes. For example, a ToString() method may have 3 or 4 lines of code, which may appear in many domain objects, and such duplicate codes can be ignored.
Duplicate code is more difficult to analyze: when you encounter a problem, you must want to solve it. One key point to solve is to locate the problem. When you call an existing method, you can easily find the source code. When you copy the code, the root of the problem may still exist elsewhere.
Code duplication is more difficult to modify: if the repeated code contains a BUG, the same BUG will appear many times. The same problem also exists in conventional modification, which needs to be modified in many places.
How to use principles
Extract duplicate codes into a method. For duplicate codes between different classes, you can put the extraction method into a tool class and use static methods.
public class Accounts
{
public static bool IsValid(string number)
{
int sum = 0;
for (int i = 0; i < number.Length; i++)
{
sum += (9 - i) * (int)char.GetNumericValue(number[i]);
}
return sum % 11 == 0;
}
}Then the CheckingAccount class becomes:
public class CheckingAccount
{
private int transferLimit = 100;
public Transfer MakeTransfer(string counterAccount, Money amount)
{
//1 check withdrawal limit
if (amoiunt.GreaterThan(this.transferLimit))
{
throw new BusinessException("Limit exceeded!");
}
//2 assume that the result is a 9-digit bank account number, which is verified by 11 test
if (Accounts.IsValid(counterAccount))
{
//3 find the opposite account and create a transfer object
CheckingAccount acct = Accounts.FindAcctByNumber(counterAccount);
Transfer result = new Transfer(this, acct, amount);
return result;
}
else
{
throw new BusinessException("Invalid account number!");
}
}
}Similarly, the SavingAccount class changes to:
public class SavingAccount
{
CheckingAccount registeredCounterAccount;
public Transfer MakeTransfer(string counterAccount, Money amount)
{
//1 assume that the result is a 9-digit bank account number, which is verified by 11 test
if (Accounts.IsValid(counterAccount))
{
//2 find the opposite account and create a transfer object
CheckingAccount acct = Accounts.FindAcctByNumber(counterAccount);
Transfer result = new Transfer(this, acct, amount);
//3 check whether the withdrawing party is a registered opposite account
if (result.getCounterAccount().Equals(this.registeredCounterAccount))
{
return result;
}
else
{
throw new BusinessException("Counter-account not registered!");
}
}
else
{
throw new BusinessException("Invalid account number!");
}
}
}At this time, the duplicate code has disappeared, but there are the following problems:
1. The duplicate code no longer exists, but the same logic still exists in the two classes.
2. Because the method of C # must be in a class, the extracted code has to be placed in another class. This class will soon become a hodgepodge of irrelevant methods, resulting in huge volume and tight coupling. There are too many irrelevant methods providing various functions in the class. In this way, other methods must know their implementation details in order to use this huge class, Cause tight coupling between each other.
To solve the above problems, extract the parent class. This refactoring technique not only extracts the code line fragment into a method, but also extracts it into a new parent class of the original class.
public class Account
{
public virtual Transfer MakeTransfer(string counterAccount, Money amount)
{
//1 assume that the result is a 9-digit bank account number, which is verified by 11 test
int sum = 0;
for (int i = 0; i < counterAccount.Length; i++)
{
sum += (9 - i) * (int)char.GetNumericValue(counterAccount[i]);
}
if (sum % 11 == 0)
{
//2 find the opposite account and create a transfer object
CheckingAccount acct = Accounts.FindAcctByNumber(counterAccount);
Transfer result = new Transfer(this, acct, amount);
return result;
}
else
{
throw new BusinessException("Invalid account number!");
}
}
}This parent class includes the logic shared by two special accounts. The following are two subclasses
public class CheckingAccount : Account
{
private int transferLimit = 100;
public override Transfer MakeTransfer(string counterAccount, Money amount)
{
//1 check withdrawal limit
if (amoiunt.GreaterThan(this.transferLimit))
{
throw new BusinessException("Limit exceeded!");
}
return base.MakeTransfer(counterAccount, amount);
}
}
public class SavingAccount : Account
{
CheckingAccount RegisteredCounterAccount { get; set; }
public override Transfer MakeTransfer(string counterAccount, Money amount)
{
Transfer result = base.MakeTransfer(counterAccount, amount);
//3 check whether the withdrawing party is a registered opposite account
if (result.getCounterAccount().Equals(this.RegisteredCounterAccount))
{
return result;
}
else
{
throw new BusinessException("Counter-account not registered!");
}
}
}For the above parent class, you can also extract part of the verification account number as follows:
public class Account
{
public virtual Transfer MakeTransfer(string counterAccount, Money amount)
{
//1 assume that the result is a 9-digit bank account number, which is verified by 11 test
if (IsValid(counterAccount))
{
//2 find the opposite account and create a transfer object
CheckingAccount acct = Accounts.FindAcctByNumber(counterAccount);
Transfer result = new Transfer(this, acct, amount);
return result;
}
else
{
throw new BusinessException("Invalid account number!");
}
}
public static bool IsValid(string number)
{
int sum = 0;
for (int i = 0; i < number.Length; i++)
{
sum += (9 - i) * (int)char.GetNumericValue(number[i]);
}
return sum % 11 == 0;
}
}Common objections:
It should be allowed to copy code directly from other code bases. Copying code from other code bases is not a problem because it does not bring duplicate code to the current system.
If this code solves the same problems in the same environment, copying the code seems to bring benefits, but if there is any of the following situations, you will encounter trouble:
1. Another code base is still under maintenance. If you just copy the code, you will not be able to make subsequent improvements to the original code.
2. If another code base is no longer maintained, you are rewriting this code base. Generally, rewriting existing code is only considered when encountering maintainability problems or technical updates. If maintainability is poor, the rewrite plan may be affected by the copied code because you have introduced code that is determined to be difficult to maintain. If it is a technology update, it may bring the limitations of the old technology to the new code base. For example, the required abstract concepts cannot be used, resulting in the inability to reuse methods effectively.
Code duplication due to minor modifications is inevitable
The system often contains some slightly different general methods. For example, some methods are only slightly different for different operating systems, different versions or different user groups, but this does not mean that repeated code is inevitable. You need to find the same part of all these codes and move it to a general parent class, Efforts should be made to distinguish the differences in code to ensure that they are clear, independent and testable.
These codes will never change
In reality, most systems will change for many reasons, and the functional requirements of the system may change due to the change of user groups, use behavior and organizational business. The organization may change in ownership, scope of responsibilities, development methods, development processes or legal requirements. The technology in the environment of the system may also change, such as the operating system, third-party libraries, frameworks or interfaces with other applications. The code itself may also be changed due to bugs, refactoring and even interface optimization.
Copying the whole file is equivalent to an additional backup. A reserved backup is established. A version control system such as Git provides a good backup mechanism. Do not put the backup file in the code base. In the future, you will not be able to distinguish which file is really used.
Unit testing can help us find problems, which is true only when the repeated code is in the same method and the unit test can cover the two pieces of code. If the duplicate code is in another method, you can only rely on the code analyzer to check whether it has changed. Otherwise, only the repeated code changes, and the unit test does not necessarily prompt failure. Therefore, we should not only rely on the results of unit tests without looking for the root cause of the problem. We can't think that all the problems will be solved in the subsequent development process, so we can turn a blind eye to them now.
The repetition of string text is inevitable and harmless. We often see in C# code that long SQL queries, XML or HTML documents in the form of string are exactly the same sometimes, but more often some parts of them appear repeatedly. For example, SQL queries with hundreds of lines are only different in sorting order, Although repeated codes here are not the logic of C# itself, they are still harmful. In fact, the solution to this kind of repeated code is very simple: extract the string into a method, and use string append and parameter methods to process variables. Use the template engine to generate HTML content through multiple files with smaller volume and no duplicate code.
SIG evaluation duplicate code
Evaluate duplicate codes. Except for the duplicate codes that are completely import statements, all category 1 codes with more than 6 lines are included, and then these duplicate codes are divided into two risk categories: redundant duplicate codes and non redundant duplicate codes. For example, there are three repetitions of 10 lines of code in the code base, two of which can be removed, and they are all considered redundant. Therefore, line i code is redundant, and another repetitive code is considered non redundant. If you want to be rated as 4 stars, the proportion of redundant code is up to 4.6%