Программная иженерия Андрей Дмитриев ©

Улучшение существующего кода

Программа Основные принципы Разработка тестов Виды рефакторингов: Реструктуризация данных Составление и перенос методов

Препосылки Хорошая структура создается не с первого раза. Внесение изменений (исправление, добавление) в существующий код сложно. Итеративная разработка часто сводится к резкому падению качества кода.

Рефакторинг Процесс изменения кода программы так, что внешнее поведение не меняется (или меняется незначительно), но улучшается его внутренняя структура. Изменение дизайна кода после того как он был написан.

Оправдание Прозрачность кода. Упрощение процесса внесения изменений. После рефакторинга программа работает точно так же. Изменения видны только специалистам, работающим с кодом. Изменяется соотношение объемов работ различных этапов процесса разработки: Проектирование и тестирование осуществляется непрерывно.

Каждый разработчик хотя бы раз использовал принципы рефакторинга в работе.

Изменение существующего проекта Пусть есть проект, в котором участвуют следующие сущности: Movie – класс, представляющий информацию о фильме. Клиент – класс, характеризующий арендатора фильма. Заказ – класс, содержащий сведения о сроке проката.

Класс Movie public class Movie { public static final int CHILDRENS = 2; public static final int REGULAR = 0; public static final int NEW_RELEASE = 1; private String _title; private int _priceCode; public Movie(String title, int priceCode) { _title = title; _priceCode = priceCode; } public int getPriceCode() { return _priceCode; } public void setPriceCode(int arg) { _priceCode = arg; } public String getTitle (){ return _title; }; } Класс, содержащий информацию о фильме.

Класс Customer class Customer { private String _name; private Vector _rentals = new Vector(); public Customer (String name){ _name = name; }; public void addRental(Rental arg) { _rentals.addElement(arg); } public String getName (){ return _name; }; Класс, содержащий информацию об арендаторе.

Класс Rental class Rental { private Movie _movie; private int _daysRented; public Rental(Movie movie, int daysRented) { _movie = movie; _daysRented = daysRented; } public int getDaysRented() { return _daysRented; } public Movie getMovie() { return _movie; } Класс, содержащий информацию об аренде.

Функциональность Customer (1/3) public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = «Выписка для " + getName() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); … Метод statement() формирует отчет о состоянии аренды клиента по каждому арендованному фильму:

Функциональность Customer (2/3) switch (each.getMovie().getPriceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.getDaysRented() > 2) thisAmount += (each.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.getDaysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.getDaysRented() > 3) thisAmount += (each.getDaysRented() - 3) * 1.5; break;} Затем подсчитывает стоимость в зависимости от типа фильма и длительности проката:

Функциональность Customer (3/3) frequentRenterPoints ++; if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints ++; result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount; } //while loop result += "Amount owed is " + String.valueOf(totalAmount) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }//end method Наконец, начисляет клиенту бонусы за активность и подводит итог:

Наблюдение Метод statement() слишком большой. Метод statement() состоит из частей, принадлежащих другим классам. Добавление нового вида фильма влечет изменения в этом методе. Создание отчета в виде HTML потребует полного переписывания метода statement().

Требование 1 Гарантировать стабильную работу программы после всех изменений.

Выделение метода Разобьем метод на несколько более коротких. Для этого найдем несвязные куски кода. Как правило они работают с локальными переменными.

Случай с переменной thisAmount switch (each.getMovie().getPriceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.getDaysRented() > 2) thisAmount += (each.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.getDaysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.getDaysRented() > 3) thisAmount += (each.getDaysRented() - 3) * 1.5; break; } Вынесем весь блок switch:

Случай с переменной thisAmount (cont.) private double amountFor(Rental each) { double thisAmount = 0; switch (each.getMovie().getPriceCode()) { case Movie.REGULAR: thisAmount += 2; if (each.getDaysRented() > 2) thisAmount += (each.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: thisAmount += each.getDaysRented() * 3; break; case Movie.CHILDRENS: thisAmount += 1.5; if (each.getDaysRented() > 3) thisAmount += (each.getDaysRented() - 3) * 1.5; break; } return thisAmount; } Локальная переменная стала локальной переменной другого метода:

Случай с переменной thisAmount (cont.) Метод statement() стал короче: public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); thisAmount = amountFor(each); frequentRenterPoints ++; if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints ++; result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount; …

Переименование переменной private double amountFor(Rental aRental) { double result = 0; switch (aRental.getMovie().getPriceCode()) { case Movie.REGULAR: result += 2; if (aRental.getDaysRented() > 2) result += (aRental.getDaysRented() - 2) * 1.5; break; case Movie.NEW_RELEASE: result += aRental.getDaysRented() * 3; break; case Movie.CHILDRENS: result += 1.5; if (aRental.getDaysRented() > 3) result += (aRental.getDaysRented() - 3) * 1.5; break; } return result;} Переименуем thisAmount в result:

Перемещение метода amountFor() Данный метод использует данные класса Rental, а не класса Customer. Имеет смысл перенести метод в класс, наиболее часто используемый данным методом.

Перемещение метода amountFor() (cont.) Плюс переименование метода и удаление параметра: 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;}

Перемещение метода amountFor() (cont.) class Customer... private double amountFor(Rental aRental) { return aRental.getCharge(); } Подключим новый метод к классу Customer для проверки корректности:

Перемещение метода amountFor() (cont.) class Customer... public String statement() { … Rental each = (Rental) rentals.nextElement(); thisAmount = amountFor(each); На : thisAmount = each.getCharge(); После этого можно удалить старый метод. Найдем места, где используется старый метод и заменим их:

Замена переменной методом Переменная thisAmount используется только для чтения. Ее можно заменить методом класса Rental. public String statement() { double thisAmount = 0; Rental each = (Rental) rentals.nextElement(); … thisAmount = each.getCharge(); … result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(thisAmount) + "\n"; totalAmount += thisAmount;

Замена переменной методом (cont.) Вычисление теперь происходит дважды: public String statement() { … result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(each.getCharge()) + "\n"; totalAmount += each.getCharge(); …

Выделение метода подсчета бонусов Данный блок зависит только от класса Rental: public String statement() { int frequentRenterPoints = 0; … frequentRenterPoints ++; if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && each.getDaysRented() > 1) frequentRenterPoints ++; …

Выделение метода подсчета бонусов (cont.) Выделим метод: class Customer... public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = "Rental Record for " + getName() + "\n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += each.getFrequentRenterPoints();

Выделение метода подсчета бонусов (cont.) Перенесем его в другой класс: class Rental... int getFrequentRenterPoints() { if ((getMovie().getPriceCode() == Movie.NEW_RELEASE) && getDaysRented() > 1) return 2; else return 1; }

Дальнейшая замена переменных вызовом метода Заменим переменную totalAmount на метод. class Customer... public String statement() { double totalAmount = 0; … while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += each.getFrequentRenterPoints(); result += "\t" + each.getMovie().getTitle()+ "\t" + String.valueOf(each.getCharge()) + "\n"; totalAmount += each.getCharge(); }

Дальнейшая замена переменных вызовом метода (cont.) Вычисление будет производиться только в конце : class Customer... public String statement() { … result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n"; result += "You earned " + String.valueOf(frequentRenterPoints) + " frequent renter points"; return result; }

Дальнейшая замена переменных вызовом метода (cont.) Новый метод должен использовать цикл, как и раньше: class Customer.. private double getTotalCharge() { double result = 0; Enumeration rentals = _rentals.elements(); while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); result += each.getCharge(); } return result;}

Замена переменной frequentRenterPoints методом Аналогичная замена для второй переменной: public String statement() { result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n"; result += "You earned " + String.valueOf(getTotalFrequentRenterPoints()) + " frequent renter points"; return result; }

Замена переменной frequentRenterPoints методом (cont.) Метод использует цикл для подсчета бонусов: private int getTotalFrequentRenterPoints(){ int result = 0; Enumeration rentals = _rentals.elements(); while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); result += each.getFrequentRenterPoints(); } return result; }

Добавление метода htmlStatement() Введение новой функциональности выглядит несложным: public String htmlStatement() { Enumeration rentals = _rentals.elements(); String result = " Rentals for " + getName() + " \n"; while (rentals.hasMoreElements()) { Rental each = (Rental) rentals.nextElement(); result += each.getMovie().getTitle()+ ": " + String.valueOf(each.getCharge()) +" \n"; } result += " You owe " + String.valueOf(getTotalCharge()) + " \n"; result += "On this rental you earned " + String.valueOf(getTotalFrequentRenterPoints()) + " frequent renter points "; return result; }

Выводы Метод должен принадлежать классу, к которому он логически относится. Избавление от временных переменных может улучшить внешний вид кода. Замена методом может увеличить количество кода. Замедление программы на этапе рефакторинга допустимо.

Перенос метода Rental.getCharge() Метод использует константы класса Movie, которые в будущем могут меняться: 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: …

Перенос метода getCharge() (cont.) Для упрощения дальнейшей разработки стоит перенести метод в класс Movie. Метод getCharge() будет вычислять стоимость на основе внешних данных: class Movie... double getCharge(int daysRented) { double result = 0; switch (getPriceCode()) { 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: …

Перенос метода getFrequentRenterPoints() Аналогично можно перенести метод из класса Rental в Movie: class Rental... int getFrequentRenterPoints() { if ((getMovie().getPriceCode() == Movie.NEW_RELEASE)&&getDaysRented() > 1) return 2; else return 1; }

Перенос метода getFrequentRenterPoints() (cont.) Метод получает внешний параметр: class Movie… int getFrequentRenterPoints(int daysRented) { if ((getPriceCode() == Movie.NEW_RELEASE)&& daysRented > 1) return 2; else return 1; }

Наследование Несколько типов фильмов можно представить в виде иерархии классов.

Замена блока switch полиморфизмом Фильм может поменять свою категорию в течение своей жизни. Стоимость поддержки объектов в актуальном состоянии при такой организации крайне сложна.

Применение шаблона Состояние или Стратегия Можно делегировать вычисление стоимости новому классу Price:

Код типа Ограничимся пока кодом типа фильма: class Movie… public Movie(String name, int priceCode) { _name = name; _priceCode = priceCode; } Или: class Movie… public Movie(String name, int priceCode) { _name = name; setPriceCode(priceCode); }

Иерархия классов цен Классы могут пока только возвращать константы: abstract class Price { abstract int getPriceCode(); } class ChildrensPrice extends Price { int getPriceCode() { return Movie.CHILDRENS; }

Иерархия классов цен (cont.) Классы могут пока только возвращать константы: class NewReleasePrice extends Price { int getPriceCode() { return Movie.NEW_RELEASE; } class RegularPrice extends Price { int getPriceCode() { return Movie.REGULAR; }

Новое поле в классе Movie class Movie... private Price _price; public int getPriceCode() { return _price.getPriceCode(); } public void setPriceCode(int arg) { switch (arg) { case REGULAR: _price = new RegularPrice(); break; case CHILDRENS: _price = new ChildrensPrice(); break; case NEW_RELEASE: _price = new NewReleasePrice(); break; default: throw new IllegalArgumentException("Incorrect Price Code"); }

getCharge() остается почти без изменений Переместим метод getCharge() из класса Movie в Price: class Price... double getCharge(int daysRented) { double result = 0; switch (getPriceCode()) { 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: …

Замена блока switch полиморфизмом Переносим части switch в методы соответствующих классов иерархии: class RegularPrice... double getCharge(int daysRented){ double result = 2; if (daysRented > 2) result += (daysRented - 2) * 1.5; return result; }

Замена блока switch полиморфизмом (cont.) Аналогично изменяем другие два класса: class ChildrensPrice double getCharge(int daysRented){ double result = 1.5; if (daysRented > 3) result += (daysRented - 3) * 1.5; return result; } class NewReleasePrice... double getCharge(int daysRented){ return daysRented * 3; }}

Вынесение общего метода Объединяем всю иерархию еще одним методом: class Price... abstract double getCharge(int daysRented);

Обобщение других методов Метод getFrequentRenterPoints(daysRented) может быть также помещен из класса Rental в Price: class Rental... int getFrequentRenterPoints(int daysRented) { if ((getPriceCode() == Movie.NEW_RELEASE) && daysRented > 1) return 2; else return 1; }

Обобщение других методов (cont.) class Price... … int getFrequentRenterPoints(int daysRented){ return 1; } class NewReleasePrice int getFrequentRenterPoints(int daysRented) { return (daysRented > 1) ? 2: 1; }

Итоговая схема

Выводы В результате рефакторинга программа получает большую наглядность. Достигается упрощение внесения изменений. Изменения сопровождаются непрерывным тестированием.

Дополнительные источники Мартин Фаулер, Рефакторинг William H. Brown Antipatterns. Refactoring software, architectures and projects in crisis.

Q&A

Спасибо! Андрей Дмитриев ©