The video store example from Episode 3 of cleancoders.com. Based upon, but not identical to, the first chapter of Martin Fowler's classic book: Refactoring.
I made some changes from the original
- Made a Maven project
- Migrated the original text written in Junit 3 to Junit5
- Very old code like written by C++ programmer
- Variables in some classes are placed at bottom
- Test class has long lines which make us scroll horizontally right
- Yuck
- Tests pass
- Break the strings in the test methods into multiple parts
- Makes methods visible all in one screen
- No scrolling to right needed
- All test methods are testing UI that is actual report
- All they need to test is the amounts calculated
- UI string test can be there in one of the tests for completeness
- Repetition of UI strings introduce regression impact
- Move all variables in all classes to top
- Add assertions with code as below
assertEquals(9.0, customer.getTotalAmount(), DELTA);
assertEquals(2, customer.getFrequentRenterPoints());
- Create the necessary methods required with IDE intellisense
- Promote variables
totalAmount
andfrequentRenterPoints
to fields, keep init in current method - Remove the string comparison in first test
- Just execute
customer.statement();
to generate them
Customer
class is just generating a statement- It has nothing to do with a customer other than maintaining a variable of
Customer customerName
- Rename
Customer
class toStatement
- Rename
name
inStatement
class tocustomerName
- Rename
statement
method togenerate
- Change all test methods to assert calculated values
- Duplicate last test to maintain one of them as testing string
- Add suffix
format
to the last method - Add suffix
Totals
to all other test methods - Keep the string comparison as is in last method
- Change
Fred
toCustomer
in customer name - Change the format method to reflect this change
- extract field for
new Movie("The Cell", Movie.NEW_RELEASE)
with namenewReleaseMovie1
- extract
new Movie("The Tigger Movie", Movie.NEW_RELEASE)
into fieldnewReleaseMovie2
- change title of
The Cell
toNew Release 1
- change title of
The Tiger Movie
toNew Release 2
- change all movies to variables and titles to generic titles
- change the format method to reflect the changes
- generate method of the
Statement
class - extract the statements which initialize to method named
initialize()
totalAmount = 0;
frequentRenterPoints = 0;
- Rename result variable to
statementText
- extract method for
createHeader()
- Change return value of
createHeader()
toMessageFormat.format()
while
loop in the generate method just adds rental lines to the statement
- Extract
while
loop in method namedcreateRentalLines()
- Remove the params and eliminate side effects
- Rename
statementText
torentalLinesText
increateRentalLines()
method - Combine footer test strings and format to multiple lines
- Extract footer logic to
createFooter()
- Rename
initialize()
method toclearTotals()
- Change return value in
createFooter()
toMessageFormat.format()
- Change the order of methods in order of calls
- Step down method of ordering methods
- Remove unused
getCustomerName()
method - Extract body of
while
loop from methodcreateRentalLines()
to method namedcreateRentalLine()
- Rename variable
rentalLinesText
torentalLineText
in methodcreateRentalLine()
- Rename parameter
each
torental
in methodcreateRentalLine()
- Extract the switch statement into method named
determineAmount()
using comment content forname
- Do not pass
thisAmount
variable - Rename variable
thisAmount
torentalAmount
in methoddetermineAmount
andcreateRentalLine
- Extract
frequentRenterPoints
login into methoddetermineFrequentRenterPoints
- Modify method to make it simpler and return value without side effect
- Extract condition in
determineFrequentRenterPoints
method into a variable - Name it
bonusIsEarned
- Rearrange all calculations at top in
createRentalLine
- Extract formatting of rental lines into method
formatRentalLines
- Remove variable declaration
- Change return value to
MessageFormat.format()
call - Inline variable for
createRentalLine()
method return value - Remove demeter law violation for
rental.getMovie().getTitle()
- In method
formatRentalLines
- Making method call like
rental.getTitle()
- Rearrange methods of Statement class for step down method
determineAmount()
method is not cohesive- It does not use any fields in statement class
- It does not belong to
Statement
class - In fact, it should belong to
Rental
class as it uses methods from that class - same true with method
determineFrequentRenterPoints
- Move the method
determineAmount()
toRental
class as public - change calls to
getMovie()
andgetDaysRented()
to field variables - Move
determineFrequentRenterPoints
method toRental
class - change calls to
getMovie()
andgetDaysRented()
to field variables
- In
Rental
classdetermineAmount()
, we use four things from movie and only one thing fromRental
- This method hence belongs to
Movie
class - Same with method
determineFrequentRenterPoints()
- Add signatures of method like follows
public double determineAmount() {
return determineAmount(daysRented);
}
public double determineAmount(int daysRented) {
...
}
- Move the method with argument to
Movie
class - Fix the rental this issue
- Same Move with
determineFrequentRenterPoints()
method toMovie
- Inline
priceCode
in both methods
determineAmount()
inMovie
is using switch- it should use polymorphic dispatch
- we have to do this without breaking code
- In
Test
class - Change the
Movie
class names to subclasses to create them with IDE - remove the second parameter from constructors as it is redundant now
- In
Movie
class fordetermineAmount()
method - use refactoring Push Members Down, keep in
Movie
asabstract
- Run with coverage and remove unused code from all subclasses
- Remove the type codes of
Movies
now as they are redundant - Change signature of constructor of
Movie
and removepriceCode
- Go to settings > Editor > Code inspection > Code style issues
- check instance field access not qualified with
this
- check instance method access not qualified with
this
- Run code analysis on
videostore
package - Fix all issues