Learning refactoring-improving existing code (1)

Continued http://blog.csdn.net/f562867448/article/details/6719460

Now we have followed the book school to reconstruct statament.

(Many books have said this: Make your code as concise as possible and reduce repeated code)

Examples in the book

Before reconstruction: Public String Statement () {double totalamount = 0; // total consumption amount. Int frequentrenterpoints = 0; // frequent customer Accumulation Point enumeration sorted als = _ sorted ALS. elements (); string result = "Marshal record for" + getname () + "\ n"; while (marshals. hasmoreelements () {double thisamount = 0; Marshal each = (Marshal) als. nextelement (); // obtain a lease record. // Determine amounts for each line switch (each. getmovie (). getpricecode () {// get the rental price of the Video Case movie. regular: // common thisamount + = 2; If (each. getdaysrented ()> 2) thisamount + = (each. getdaysrented ()-2) * 1.5; break; Case movie. new_release: // new thisamount + = each. getdaysrented () * 3; break; Case movie. childrens: // child. Thisamount ++ = 1.5; If (each. getdaysrented ()> 3) thisamount + = (each. getdaysrented ()-3) * 1.5; break;} // Add frequent renter points (accumulated frequent customer points. Frequentrenterpoints ++; // Add bonus for a two day new release marshal if (each. getmovie (). getpricecode () = movie. new_release) & each. getdaysrented ()> 1) frequentrenterpoints ++; // show figures for this lease Al (display this lease record) Result + = "\ t" + each. getmovie (). gettitle () + "\ t" + String. valueof (thisamount) + "\ n"; totalamount + = thisamount;} // Add footer lines (printed at the end) Result + = "amount owed is" + String. valueof (totalamount) + "\ n"; Result + = "you earned" + String. valueof (frequentrenterpoints) + "Frequent renter points"; return result ;}

After the first modification:

Public String Statement () {double totalamount = 0; int frequentrenterpoints = 0; enumeration grants als = _ temporary ALS. elements (); string result = "Marshal record for" + getname () + "\ n"; while (marshals. hasmoreelements () {double thisamount = 0; Marshal each = (Marshal) als. nextelement (); thisamount = amountfor (each); // calculate the rent fee. // Add frequent renter points frequentrenterpoints ++; // Add bonus for a two day new release marshal if (each. getmovie (). getpricecode () = movie. new_release) & each. getdaysrented ()> 1) frequentrenterpoints ++; // show figures for this sort Al result + = "\ t" + each. getmovie (). gettitle () + "\ t" + String. valueof (thisamount) + "\ n"; totalamount + = thisamount;} // Add footer lines result + = "amount owe D is "+ String. valueof (totalamount) + "\ n"; Result + = "you earned" + String. valueof (frequentrenterpoints) + "Frequent renter points"; return result;} private int amountfor (billing Al each) {// calculates a rent fee. Int thisamount = 0; Switch (each. getmovie (). getpricecode () {Case movie. regular: // common thisamount + = 2; If (each. getdaysrented ()> 2) thisamount + = (each. getdaysrented ()-2) * 1.5; break; Case movie. new_release: // new thisamount + = each. getdaysrented () * 3; break; Case movie. childrens: // child. Thisamount + = 1.5; If (each. getdaysrented ()> 3) thisamount + = (each. getdaysrented ()-3) * 1.5; break;} return thisamount;

 

Return thisamount;

In this example, the rent fee is written as a private method!

We recommend that you test the modification without any modification, so that you can find it quickly even if there is an error. Don't think you are a genius. A very small problem may lead to a major bug behind you. So doing more tests won't waste your time.

 

The author continued to repair the private int amountfor (rental each) and changed the incoming parameter each to the arental modification. I think it is necessary to let people see the object pointed to by the parameter at a glance, easier to understand! It is only in a short method!

Author's feelings in the book

I feel that this amount should be calculated in Marshal class, and the Marshal class should be changed

class Rental...  double getCharge() {      double result = 0;      switch (getMovie().getPriceCode()) {          case Movie.REGULAR:              result += 2;              if (getDaysRented() > 2)                  result += (getDaysRented() - 2) * 1.5;              break;          case Movie.NEW_RELEASE:              result += getDaysRented() * 3;              break;          case Movie.CHILDRENS:              result += 1.5;              if (getDaysRented() > 3)                  result += (getDaysRented() - 3) * 1.5;              break;      }      return result;  }

 

Refining [frequent customer point calculation]

Retrieve temporary variables

I will not copy these books.

 

The last step uses polymorphism. In this example, movie has different types. If we use a class for each type of movie, it is easier to use each class extends movie class. In this way, add public utilities to the parent class, and add methods of different movie types to their respective classes! In this way, it will be much easier to maintain in the future!