I’ve written an article about finding bad smells in code, notes on Refactoring (Part 1). A quick summary of the key points of refactoring and the author’s “bad smell in code”.

This summary is rather abstract, and I will write a series of articles to give you a concrete feeling of the charm of small refactoring through case studies. The example is also used in chapter 1 of Martin Fowler’s Book Refactoring. But the cases in this book are written in Java, so I’ll change it to C++ programs. Hopefully this will help C++ programmers understand refactoring better.

The starting point

Consider the implementation of a rental store needs to use the program, the requirements are:

  • Calculate the amount of each customer and print out the details.
  • Operator input: which movies the customer rents and how long the rental period, the program according to the rental time and film type to calculate the cost.
  • The films fall into three categories: regular, children’s and new.
  • Credits are calculated for members, depending on whether the film type is new or not.

This is a UML class diagram of a simple implementation of the program.

Here is the code for each class in the UML class diagram:

This is a Movie.

Movie is just a simple, pure data class.

enum PriceCode
{
	REGULAR = 0,
	NEW_RELEASE = 1, 
	CHILDREN = 2
};

class Movie 
{
public:
	Movie() {}
	Movie(const string& title, int price_code) {
		title_ = title;
		price_code_ = price_code;
	}

	int GetPriceCode(a) { return price_code_; }
	void SetPriceCode(int price_code) { price_code_ = price_code; }
	string GetTitle(a) { return title_; }

private:
	string title_;
	int price_code_;
};
Copy the code

Rental (Rental)

Rental means that a customer rents a movie.

class Rental
{
public:
	Rental() {}
	Rental(const Movie& movie, int days_rentaed) {
		movie_ = movie;
		days_rented_ = days_rentaed;
	}

	int GetDaysRented(a) { return days_rented_; }
	Movie GetMovie(a) { return movie_; }

private:
	Movie movie_;
	int days_rented_;
};
Copy the code

Customer

The Customer class is used to represent customers. Just like any other class, it has data and corresponding accessors. Customer also provides a function, Statement (), that generates the bill.

class Customer
{
public:
	Customer(const string& name) {
		name_ = name;
	}

	void AddRental (const Rental& arg) { rentals_.emplace_back(arg); }
	string GetName(a) { return name_; }
	string Statement(a) {
		double total_amount = 0;
		int frequent_renter_points = 0;
		string result = "Rental Record for " + GetName() + "\n";
		for (auto& each : rentals_) {
			double this_amount = 0;
			
			// determine amounts for each line
			switch (each.GetMovie().GetPriceCode())
			{
			case PriceCode::REGULAR:
				this_amount += 2;
				if (each.GetDaysRented() > 2)
					this_amount += (each.GetDaysRented() - 2) * 1.5;
				break;
			case PriceCode::NEW_RELEASE:
				this_amount += each.GetDaysRented() * 3;
				break;
			case PriceCode::CHILDREN:
				this_amount += 1.5;
				if (each.GetDaysRented() > 3)
					this_amount += (each.GetDaysRented() - 3) * 1.5;
				break;
			}

			// add frequent renter points
			frequent_renter_points ++;
			// add bonus for a two day new release rental
			if (each.GetMovie().GetPriceCode() == PriceCode::NEW_RELEASE &&
				each.GetDaysRented() > 1)
				frequent_renter_points++;

			
			// show figures for this rental
			result += "\t" + each.GetMovie().GetTitle() + "\t" +
				to_string(this_amount) + "\n";
			total_amount += this_amount;
		}

		// add footer lines
		result += "Amount owed is " + to_string(total_amount) + "\n";
		result += "You earned " + to_string(frequent_renter_points) +
			" frequent renter points";
		
		return result;
	}

private:

	string name_;
	vector<Rental> rentals_;
};
Copy the code

For our initial version of the program, first of all, we can be sure that it works properly, without bugs. If this is a test program, or if the program is as it is and will never be changed, then we can even say it is perfect because we implemented the desired functionality quickly and without bugs. But it’s easy to smell the “bad code smell” of Long Methods, as summarized in refactoring. When all the business logic is piled up in this long function, it makes the program hard to modify, hard to expand, and possibly worse. At this point, it becomes obvious that we need to refactor.

If you find that you need to add a feature to your program and the structure of the code doesn’t make it easy to do so, refactor the program to make it easier to add features, and then add features.

Refactoring Step 1

Whenever I do a refactoring, the first step is always the same: I have to set up a reliable test environment for the code I’m about to change.

The author emphasizes the importance of unit testing, because people can always make mistakes and refactoring can cause unexpected bugs, so it is necessary to unit test the interface parts of the original function. But most of the time in real projects we ignore this because of the urgency of the need. Here, I’ve added a simple test code to the main function.

int main() {
	Movie movie1{ "Pulp Fiction", PriceCode::REGULAR };
	Movie movie2{ "Joker", PriceCode::NEW_RELEASE };
	Movie movie3{ "Peppa Pig", PriceCode::CHILDREN };

	Rental rental1{ movie1, 3 };
	Rental rental2{ movie2, 3 };
	Rental rental3{ movie3, 3 };

	Customer customer{ "Caesar" };
	customer.AddRental(rental1);
	customer.AddRental(rental2);
	customer.AddRental(rental3);
	cout << customer.Statement().c_str() << endl;

	return 0;
}
Copy the code

The running results are shown in the figure. Then, after each small refactoring, we run the test code to see if the results are the same as the original results, and if not, we have to check the refactoring code to see if any new bugs have been introduced.

Thinking summary

  • The first step in a refactoring should always be to write a piece of unit test code to ensure that the refactoring does not introduce new bugs.

This series of articles

Find the bad smell in the code — Notes from Refactoring (Part 1)

Refactoring, first case (C++ version) – the original program

Refactoring, first case (C++) — breaking down and reorganizing Statement()

Refactoring, first case (C++) — using polymorphism to replace conditional logic with price