Download Moving The Amount Calculation-Methods Of Software Engineering-Lecture Notes and more Study notes Software Engineering in PDF only on Docsity!
Message Chains
- a client asks an object for another object and then asks that object for another object etc. Bad because client depends on the structure of the navigation Middle Man
- If a class is delegating more than half of its responsibilities to another class, do you really need it? Inappropriate Intimacy
- Pairs of classes that know too much about each other‘s private details Alternative Classes with Different Interfaces
- Symptom: Two or more methods do the same thing but have different signature for what they do Incomplete Library Class
- A framework class doesn‘t do everything you need Data Class
- These are classes that have fields, getting and setting methods for the fields, and nothing else; they are data holders, but objects should be about data AND behavior Refused Bequest
- A subclass ignores most of the functionality provided by its superclass Comments (!)
- Comments are sometimes used to hide bad code
- ―…comments often are used as a deodorant‖ (!)
Breaking a Method
We have already seen example of duplicate code. We now look at another simple example of long method. Although, in this case, the code is not really long, it however demonstrates how longer segments of code can be broken into smaller and more manageable (may be more reusable as well) code segments.
The following code segment sorts an array of integers using ―selection sort‖ algorithm.
for (i=0; i < N-1; i++) { min = i; for (j = i; j < N; j++) if (a[j] < a[min]) min = j; temp = a[i]; a[i] = a[min]; a[min] = temp; }
We break it into smaller fragments by making smaller functions out of different steps in the algorithm as follows:
int minimum (int a[ ], int from, int to) { int min = from;
for (int i = from; i <= to; i++) if (a[i] < a[min]) min = i; return min; }
void swap (int &x, int &y) { int temp = x; x = y; y = temp; }
The sort function now becomes simpler as shown below.
for (i=0; i < N-1; i++) { min = minimum (a, i, N-1); swap(a[i], a[min]); }
It can be seen that it is now much easier to understand the code and hence is easier to maintain. At the same time we have got two separate reusable functions that can be used elsewhere in the code.
A slightly more involved example (It has mostly been adapted from Fowler’s introduction to refactoring which is freely available on the web)
Let us consider a simple program for a video store. It has three classes: Movie, Rental, and Customer. Program is told which movies a customer rented and for how long and it then calculates the charges and the Frequent renter points. Customer object can print a statement (in ASCII).
Here is the code for the classes. DomainObject is a general class that does a few standard things, such as hold a name.
public class DomainObject {
public DomainObject (String name) { _name = name; };
public DomainObject () {};
public String name () { return _name; };
public String toString() { return _name;
private String _serialNumber; private Movie _movie; }
The rental class represents a customer renting a movie.
class Rental extends DomainObject { public int daysRented() { return _daysRented; } public Tape tape() { return _tape; } private Tape _tape; public Rental(Tape tape, int daysRented) { _tape = tape; _daysRented = daysRented; } private int _daysRented; }
The customer class represents the customer. So far all the classes have been dumb encapsulated data. Customer holds all the behavior for producing a statement in its statement() method.
class Customer extends DomainObject { public Customer(String name) { _name = name; } public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + name() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement();
//determine amounts for each line switch (each.tape().movie().priceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.daysRented() > 2) thisAmount += (each.daysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.daysRented() * 3; break;
case Movie.CHILDRENS: thisAmount += 1.5; if (each.daysRented() > 3) thisAmount += (each.daysRented() - 3) * 1.5; break;
} totalAmount += thisAmount;
// add frequent renter points frequentRenterPoints ++; // add bonus for a two day new release rental if ((each.tape().movie().priceCode() == Movie.NEW_RELEASE) && each.daysRented() > 1) frequentRenterPoints ++;
//show figures for this rental result += "\t" + each.tape().movie().name()+ "\t" + String.valueOf(thisAmount) + "\n";
} //add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result;
} public void addRental(Rental arg) { _rentals.addElement(arg); } public static Customer get(String name) { return (Customer) Registrar.get("Customers", name); } public void persist() { Registrar.add("Customers", this); } private Vector _rentals = new Vector(); }
What are your impressions about the design of this program? I would describe as not well designed, and certainly not object-oriented. For a simple program is this, that does not really matter. Thereís nothing wrong with a quick and dirty simple program. But if we imagine this as a fragment of a more complex system, then I have some real problems with this program. That long statement routine in the Customer does far too much. Many of the things that it does should really be done by the other classes.
This is really brought out by a new requirement, just in from the users, they want a similar statement in html. As you look at the code you can see that it is impossible to reuse any of the behavior of the current statement() method for an htmlStatement(). Your only recourse is to write a whole new method that duplicates much of the behavior of
break;
} totalAmount += thisAmount;
// add frequent renter points frequentRenterPoints ++; // add bonus for a two day new release rental if ((each.tape().movie().priceCode() == Movie.NEW_RELEASE) && each.daysRented() > 1) frequentRenterPoints ++;
//show figures for this rental result += "\t" + each.tape().movie().name()+ "\t" + String.valueOf(thisAmount) + "\n";
} //add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result;
}
This looks like it would make a good chunk to extract into its own method. When we extract a method, we need to look in the fragment for any variables that are local in scope to the method we are looking at, that local variables and parameters. This segment of code uses two: each and thisAmount. Of these each is not modified by the code but thisAmount is modified. Any non-modified variable we can pass in as a parameter. Modified variables need more care. If there is only one we can return it. The temp is initialized to 0 each time round the loop, and not altered until the switch gets its hands on it. So we can just assign the result. The extraction looks like this.
public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + name() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement();
//determine amounts for each line thisAmount = amountOf(each); totalAmount += thisAmount;
// add frequent renter points frequentRenterPoints ++; // add bonus for a two day new release rental
if ((each.tape().movie().priceCode() == Movie.NEW_RELEASE) && each.daysRented() > 1) frequentRenterPoints ++;
//show figures for this rental result += "\t" + each.tape().movie().name()+ "\t" + String.valueOf(thisAmount) + "\n";
} //add footer lines result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result;
}
private int amountOf(Rental each) { int thisAmount = 0; switch (each.tape().movie().priceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.daysRented() > 2) thisAmount += (each.daysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.daysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.daysRented() > 3) thisAmount += (each.daysRented() - 3) * 1.5; break;
} return thisAmount; }
When I did this the tests blew up. A couple of the test figures gave me the wrong answer. I was puzzled for a few seconds then realized what I had done. Foolishly I had made the return type of amountOf() int instead of double.
private double amountOf(Rental each) { double thisAmount = 0; switch (each.tape().movie().priceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.daysRented() > 2) thisAmount += (each.daysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE:
Lecture No. 43
Moving the amount calculation
As I look at amountOf, I can see that it uses information from the rental, but does not use information from the customer. This method is thus on the wrong object, it should be moved to the rental. To move a method you first copy the code over to rental, adjust it to fit in its new home and compile.
Class Rental double charge() { double result = 0; switch (tape().movie().priceCode()) { case Movie.REGULAR: result += 2; if (daysRented() > 2) result += (daysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: result += daysRented() * 3; break; case Movie.CHILDRENS: result += 1.5; if (daysRented() > 3) result += (daysRented() - 3) * 1.5; break;
} return result; }
In this case fitting into its new home means removing the parameter.
The next step is to find every reference to the old method, and adjusting the reference to use the new method. In this case this step is easy as we just created the method and it is in only one place. In general, however, you need to do a find across all the classes that might be using that method.
public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + name() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement();
//determine amounts for each line thisAmount = each.charge();
