Reconstruction C # implementation (3) overall reconstruction of Statement

1. Rename the local variable of AmountFor
First, let's take a look at the AmountFor method we extracted earlier:
[Csharp] view plaincopy
Public double AmountFor (Marshal)
{
Double thisAmount = 0;
Switch (marshal. Movie. PriceCode)
{
Case Movie. REGULAR:
ThisAmount + = 2;
If (marshal. DaysRented> 2)
ThisAmount + = (marshal. DaysRented-2) * 1.5;
Break;
Case Movie. NEW_RELEASE:
ThisAmount + = marshal. DaysRented * 3;
Break;
Case Movie. CHILDRENS:
ThisAmount + = 1.5;
If (marshal. DaysRented> 3)
ThisAmount + = (marshal. DaysRented-3) * 1.5;
Break;
}
Return thisAmount;
}
The method has a local variable: thisAmount. In fact, this name is in the Statement method. For the AmountFor method, there is only one amount that corresponds to the marshal passing in the parameter. Therefore, adding the modification of this is a superfluous practice. I do not agree with the new name (result) of the local variable in refactoring. The word "result" is too broad and many other data have mentioned this problem, for example, avoid the use of "omnipotent" words such as result, item, and manager. Therefore, the improved code is as follows:
[Csharp] view plaincopy
Private double AmountFor (Marshal)
{
Double amount = 0;
Switch (marshal. Movie. PriceCode)
{
Case Movie. REGULAR:
Amount + = 2;
If (marshal. DaysRented> 2)
Amount + = (marshal. DaysRented-2) * 1.5;
Break;
Case Movie. NEW_RELEASE:
Amount + = marshal. DaysRented * 3;
Break;
Case Movie. CHILDRENS:
Amount + = 1.5;
If (marshal. DaysRented> 3)
Amount + = (marshal. DaysRented-3) * 1.5;
Break;
}

Return amount;
}
As mentioned in the book, do not ignore this tiny refactoring, which can indeed reduce the difficulty of understanding the code. Imagine if a friend wants to say "Merry Christmas" to you and say "Happy New Year", this may not be a big problem in Western countries, however, it is difficult for Chinese people to associate two things. Therefore, it is important to let your name exactly represent its meaning. In my coding work, even more than 15% of the time may be spent on naming classes, methods, fields, and other content. Is it a waste of time? No!
Ii. Transfer the AmountFor Method to the marshal class
Refactoring: The current AmountFor does not use any attributes, fields, or methods of the Customer class. It is not so important in the Customer method, this implies that the method should not belong to a specific signal of this type, and the Marshal class is its geographic location. The reconstruction process is as follows:
1. Create a Charge attribute in the marshal class and copy the code in AmountFor:
[Csharp]
Public double Charge
{
Get
{
Double amount = 0;
Switch (Movie. PriceCode)
{
Case Movie. REGULAR:
Amount + = 2;
If (DaysRented> 2)
Amount + = (DaysRented-2) * 1.5;
Break;
Case Movie. NEW_RELEASE:
Amount + = DaysRented * 3;
Break;
Case Movie. CHILDRENS:
Amount + = 1.5;
If (DaysRented> 3)
Amount + = (DaysRented-3) * 1.5;
Break;
}
 
 
Return amount;
}
}
2. Clear the AmountFor method body of the original Customer, but retain the Declaration and empty function body.
[Csharp]
Private double AmountFor (Marshal)
{
Return 0;
}
3. Change the AmountFor implementation of Customer:
[Csharp]
Private double AmountFor (Marshal)
{
Return marshal. Charge;
}
4. Run the test project. If the test project fails, check and modify the code. Until it passes
5. Modify the Statement method so that it does not call the AmountFor method of its class to obtain the Amount of Marshal. Instead, it directly asks marshal:
[Csharp]
Public string Statement ()
{
Double totalAmount = 0;
Int frequentRenterPoints = 0;
String result = "Marshal Record for" + Name + "\ n ";
Foreach (Marshal in marshals)
{
Double thisAmount = marshal. Charge;
 
// Add frequent renter points
FrequentRenterPoints ++;
// Add bonus for a two day new release marshal
If (marshal. Movie. PriceCode = Movie. NEW_RELEASE &&
Marshal. DaysRented> 1) frequentRenterPoints ++;

// Show figures for this marshal
Result + = "\ t" + marshal. Movie. Title + "\ t" + thisAmount. ToString () + "\ n ";
TotalAmount + = thisAmount;
}
// Add footer lines
Result + = "Amount owed is" + totalAmount. ToString () + "\ n ";
Result + = "You earned" + frequentRenterPoints. ToString () + "frequent renter points ";
Return result;
}
6. Run the test. If the test fails, check and modify the code until it passes.
7. Reconstruction completed
It should be noted that, when migrating the content of the AmountFor method to marshal, it does not follow the original name, but uses the Charge name, mainly because for the Customer, this value is the Amount of Marshal, which is more appropriate for Marshal itself. This is a descriptive issue and is easily overlooked by many programmers.

3. Remove thisAmount local variables
Even if it is just a slightly complex algorithm, it usually contains a lot of intermediate and auxiliary computing volumes. These complicated variables are intertwined when the entire operation process is heap in a method. If there is only one intermediate volume, the algorithm can run normally. Why not remove it? Many programmers and mathematicians have this kind of extreme hobby, isn't it? In fact, it cannot be said that it is a hobby. A smaller variable means that the algorithm is simpler, and the simpler the algorithm can bring more sense of accomplishment or even sense of honor to these people. Aside from the mental benefits, leaving one variable alone makes this method easier to maintain. For the Statement method, in each loop, the thisAmount local variable is assigned only once, and twice is used. The initial value is Marshal. Charge. Therefore, naturally, all calls that use the thisAmount attribute instead of the marshal. Charge attribute should be equivalent:
[Csharp]
Public string Statement ()
{
Double totalAmount = 0;
Int frequentRenterPoints = 0;
String result = "Marshal Record for" + Name + "\ n ";
Foreach (Marshal in marshals)
{
// Add frequent renter points
FrequentRenterPoints ++;
// Add bonus for a two day new release marshal
If (marshal. Movie. PriceCode = Movie. NEW_RELEASE &&
Marshal. DaysRented> 1) frequentRenterPoints ++;

// Show figures for this marshal
Result + = "\ t" + marshal. Movie. Title + "\ t" + marshal. Charge. ToString () + "\ n ";
TotalAmount + = marshal. Charge;
}
// Add footer lines
Result + = "Amount owed is" + totalAmount. ToString () + "\ n ";
Result + = "You earned" + frequentRenterPoints. ToString () + "frequent renter points ";
Return result;
}
Aside from the possible efficiency problems mentioned in the book, I prefer to understand This refactoring from the semantic perspective. The preceding example uses marshal. Charge twice, which can be described as follows:
1. Give me a text of the Charge value of the marshal object. I Want To concatenate it with the title of the video to indicate the description text that the video spends.
2. The Charge value of a marshal object is added to the total cost.
In the two descriptions, the concept of "Charge Value of the marshal object" is used directly, instead of "giving me a variable, which stores the Charge value of a marshal object ", this is a typical way of thinking for programmers. It does not help to understand the nature of algorithms, but makes it easier for people who view code to fall into the Implementation Details of algorithms. So when you need to use a concept, use it directly. Of course, the real problem is that you can well summarize this concept-refactoring has such an effect.
Note: Do not forget to run the unit test after reconstruction.

4. Extract and migrate FrequentRenterPoint computing to the marshal class
In the simplified Statement method, we can clearly see that it also contains the computation of FrequentRenterPoint for every Marshal object. This is almost the same as the Charge of Marshal. Therefore, you can use the same method to handle it:
1. in the marshal type, create a new attribute FrequentRenterPoints and copy the computing logic in the Statement method to this attribute. Note that you should remove the quote of the marshal object because it belongs to the marshal class now:
[Csharp]
Public int FrequentRenterPoints
{
Get
{
Int frequentRenterPoints = 0;
 
// Add frequent renter points
FrequentRenterPoints ++;
// Add bonus for a two day new release marshal
If (Movie. PriceCode = Movie. NEW_RELEASE &&
DaysRented> 1) frequentRenterPoints ++;
 
 
Return frequentRenterPoints;
}
}
2. Run the unit test. If the unit test fails, check, debug, and modify it until it passes.
3. The implementation of this attribute is very "stupid". The actual expression is that if the condition is met, the score is 2 points. Otherwise, the score is 1 point. Therefore, it can be reconstructed:
[Csharp]
Public int FrequentRenterPoints
{
Get
{
If (Movie. PriceCode = Movie. NEW_RELEASE & DaysRented> 1)
{
Return 2;
}
Else
{
Return 1;
}
}
}
You can use the simpler three-wood OPERATOR :? To replace if-else, but the latter is generally easier to understand.
4. Run the unit test. If the unit test fails, check, debug, and modify it until it passes.
5. Replace the frequentRenterPoints calculation in the original Statement and use the FrequentRentalPoints attribute of Marshal for calculation:
[Csharp]
Public string Statement ()
{
Double totalAmount = 0;
Int frequentRenterPoints = 0;
String result = "Marshal Record for" + Name + "\ n ";
Foreach (Marshal in marshals)
{
FrequentRenterPoints + = marshal. FrequentRenterPoints;

// Show figures for this marshal
Result + = "\ t" + marshal. Movie. Title + "\ t" + marshal. Charge. ToString () + "\ n ";
TotalAmount + = marshal. Charge;
}
// Add footer lines
Result + = "Amount owed is" + totalAmount. ToString () + "\ n ";
Result + = "You earned" + frequentRenterPoints. ToString () + "frequent renter points ";
Return result;
}
6. Run the unit test. If the unit test fails, check, debug, and modify it until it passes.
7. Reconstruction completed

5. Calculate the total cost and total points
Now it is easy to see that in addition to the report text generation, the Statement method mainly implements two things: calculate the total cost, calculate the total points, and now they are mixed together. However, we can extract them into two independent query processes (for C #, they can be two attributes) and name them TotalCharge and totalw.alpoints respectively:
1. Add a property TotalCharge to the Customer class as follows:
[Csharp]
Private double TotalCharge
{
Get
{
Double sum = 0;
Foreach (Marshal aRental in marshals)
{
Sum + = aRental. Charge;
}
 
Return sum;
}
}
2. Remove all the local variables and processes involved in the total cost in the Statement method and change them to the call to TotalCharge:
[Csharp]
Public string Statement ()
{
Int frequentRenterPoints = 0;
String result = "Marshal Record for" + Name + "\ n ";
Foreach (Marshal in marshals)
{
FrequentRenterPoints + = marshal. FrequentRenterPoints;

// Show figures for this marshal
Result + = "\ t" + marshal. Movie. Title + "\ t" + marshal. Charge. ToString () + "\ n ";
}
// Add footer lines
Result + = "Amount owed is" + TotalCharge. ToString () + "\ n ";
Result + = "You earned" + frequentRenterPoints. ToString () + "frequent renter points ";
Return result;
}
3. Run the unit test. If the unit test fails, check, debug, and modify it until it passes.
4. Add the TotalRenterPoints attribute to the Customer class as follows:
[Csharp]
Private int TotalRenterPoints
{
Get
{
Int points = 0;
Foreach (Marshal aRental in marshals)
{
Points + = aRental. FrequentRenterPoints;
}
 
Return points;
}
}
5. Remove all the local variables and processes related to the total points in the Statement method and change them to the call to TotalRenterPoints:
[Csharp]
Public string Statement ()
{
String result = "Marshal Record for" + Name + "\ n ";
Foreach (Marshal in marshals)
{
// Show figures for this marshal
Result + = "\ t" + marshal. Movie. Title + "\ t" + marshal. Charge. ToString () + "\ n ";
}
// Add footer lines
Result + = "Amount owed is" + TotalCharge. ToString () + "\ n ";
Result + = "You earned" + TotalRenterPoints. ToString () + "frequent renter points ";
Return result;
}
6. Run the unit test. If the unit test fails, check, debug, and modify it until it passes.
7. Rebuild www.2cto.com

So far, let's review the Statement method. Which one is easier to see the logic it contains than the original version? At the very least, I will take a look at the next version and know that it works as follows:
In general, it generates a report text, which generates a report header with the user name, and generates a title + expense record for each marshal; the Footer of the report contains two parts: the total cost of the customer and the total points that the customer can earn.

As for many people, the extracted TotalCharge and TotalRenterPoints will bring about two additional orders als traversal, so that there is a problem of reduced efficiency. Refer to the content in refactoring.

Author: virtualxmars