Homework SourseI have three java class and I need to determined a piece of
I have three java class and I need to determined a piece of the bed smelled code, also refactoring that resolves the smells.
I know the bad smell in customer class will be the \" switch \" statement, but how do I refactoring the code ?
Customer.java class ----------------------------------------------------------------------------------------
public class Customer {
private String name;
private ArrayList<Rental> rentals = new ArrayList<Rental>();
public Customer (String name) {
this.name = name;
}
public void addRent(Rental unit) {
rentals.add(unit);
}
public String getCustomerName() {
return name;
}
public String generateStatement() {
double totalAmount = 0;
int frequentRenterPoints = 0;
String result = \"Rental Record for \" + getCustomerName() + \"\ \";
for(Rental rentalUnit : rentals) {
double thisAmount = 0;
// determine amounts for each line
switch (rentalUnit.getMovie().getPriceCode()) {
case Movie.REGULAR:
thisAmount += 2;
if (rentalUnit.getDaysRented() > 2)
thisAmount += (rentalUnit.getDaysRented() - 2) * 1.5;
break;
case Movie.NEW_RELEASE:
thisAmount += rentalUnit.getDaysRented() * 3;
break;
case Movie.CHILDRENS:
thisAmount += 1.5;
if (rentalUnit.getDaysRented() > 3)
thisAmount += (rentalUnit.getDaysRented() - 3) * 1.5;
break;
}
// add frequent renter points
frequentRenterPoints++;
// add bonus for a two day new release rental
if ((rentalUnit.getMovie().getPriceCode() == Movie.NEW_RELEASE) && (rentalUnit.getDaysRented() > 1))
frequentRenterPoints++;
// show figures for this rental
result += \"\\t\" + rentalUnit.getMovie().getMovieTitle() +
\"\\t\" + String.valueOf(thisAmount) + \"\ \";
totalAmount += thisAmount;
}
// add footer lines
result += \"Amount owed is \" + String.valueOf(totalAmount) + \"\ \";
result += \"You earned \" + String.valueOf(frequentRenterPoints) +
\" frequent renter points\";
return result;
}
}
Movie class -------------------------------------------------------------------------------------------------
public class Movie {
public static final int REGULAR = 0;
public static final int NEW_RELEASE = 1;
public static final int CHILDRENS = 2;
private String title;
private int priceCode;
public Movie(String title, int code) {
this.title = title;
this.priceCode = code;
}
public int getPriceCode() {
return priceCode;
}
public void setPriceCode(int arg) {
priceCode = arg;
}
public String getMovieTitle() {
return title;
}
}
Rental class -----------------------------------------------------------------------------------------
public class Rental {
private Movie movie;
private int daysRented;
public Rental(Movie movie, int daysRented) {
this.movie = movie;
this.daysRented = daysRented;
}
public int getDaysRented() {
return daysRented;
}
public Movie getMovie() {
return movie;
}
}
Solution
I have refactored this code and moved switch statement to rental class. It is the responsibility of rental class to give amount and frequent pointer. Customer class should not be responsible for any business logic.
I also added enum of PriceCode. You should use this enum instead of constants.
Customer.java
import java.util.ArrayList;
public class Customer {
private String name;
private ArrayList<Rental> rentals = new ArrayList<Rental>();
public Customer (String name) {
this.name = name;
this.rentals = new ArrayList<Rental>(); //This is done to avoid NullPointerException
}
public void addRent(Rental unit) {
rentals.add(unit);
}
public String getCustomerName() {
return name;
}
public String generateStatement() {
double totalAmount = 0;
int frequentRenterPoints = 0;
String result = \"Rental Record for \" + getCustomerName() + \"\ \";
for(Rental rentalUnit : rentals) {
double thisAmount = rentalUnit.getAmount();
// add frequent renter points
frequentRenterPoints += rentalUnit.getFrequentPointer();
// show figures for this rental
result += \"\\t\" + rentalUnit.getMovie().getMovieTitle() +
\"\\t\" + String.valueOf(thisAmount) + \"\ \";
totalAmount += thisAmount;
}
// add footer lines
result += \"Amount owed is \" + String.valueOf(totalAmount) + \"\ \";
result += \"You earned \" + String.valueOf(frequentRenterPoints) +
\" frequent renter points\";
return result;
}
}
Movie.java
public class Movie {
public static final int REGULAR = 0;
public static final int NEW_RELEASE = 1;
public static final int CHILDRENS = 2;
private String title;
private PriceCode priceCode;
public Movie(String title, PriceCode code) {
this.title = title;
this.priceCode = code;
}
public PriceCode getPriceCode() {
return priceCode;
}
public void setPriceCode(PriceCode arg) {
priceCode = arg;
}
public String getMovieTitle() {
return title;
}
}
Rental.java
public class Rental {
private Movie movie;
private int daysRented;
public Rental(Movie movie, int daysRented) {
this.movie = movie;
this.daysRented = daysRented;
}
public int getDaysRented() {
return daysRented;
}
public Movie getMovie() {
return movie;
}
public double getAmount() {
// determine amounts for each line
double thisAmount = 0.0;
switch (getMovie().getPriceCode()) {
case REGULAR:
thisAmount += 2;
if (getDaysRented() > 2)
thisAmount += (getDaysRented() - 2) * 1.5;
break;
case NEW_RELEASE:
thisAmount += getDaysRented() * 3;
break;
case CHILDRENS:
thisAmount += 1.5;
if (getDaysRented() > 3)
thisAmount += (getDaysRented() - 3) * 1.5;
break;
}
return thisAmount;
}
public int getFrequentPointer()
{
int frequentPointer = 1;
if ((getMovie().getPriceCode() == PriceCode.NEW_RELEASE) && (getDaysRented() > 1))
frequentPointer++;
return frequentPointer;
}
}
PriceCode.java
public enum PriceCode {
REGULAR,NEW_RELEASE,CHILDRENS;
}
