Refactoring на малки стъпки

Код на:JavaC#PythonJavaScript

Накратко: Refactoring е промяна на структурата на кода, без да се сменя поведението му, на малки назовани стъпки като Rename и Extract Method, със зелени тестове след всяка.

Проблем

Фактурата, която никой не иска да пипне

Онлайн магазинът печата фактури с метода Invoices.inv. Работи от две години:

static String inv(String n, String street, String city, String zip, List<Item> items, String type) {
    double t = 0;
    for (Item i : items) {
        t = t + i.price() * i.qty();
    }
    double d = 0;
    if (type.equals("gold")) {
        d = t * 0.05;
    } else if (type.equals("company")) {
        if (t >= 500) {
            d = t * 0.10;
        }
    }
    String s = "Фактура: " + n + "\n";
    s = s + street + ", " + zip + " " + city + "\n";
    s = s + String.format("Сума: %.2f €\n", t);
    s = s + String.format("Отстъпка: %.2f €\n", d);
    s = s + String.format("Общо с ДДС: %.2f €", afterDiscount(t, d) * 1.20);
    return s;
}

static double afterDiscount(double t, double d) {
    return t - d;
}

Какво е t? А d? Шест параметъра подред, четири от тях текст: който размени града и пощенския код, получава фактура с „1504 София“ на грешното място, и компилаторът не казва нищо. Веригата от if по вида клиент расте: следващия месец идват партньори с 3%. А afterDiscount прави едно изваждане, но те кара да скочиш на друго място, за да го видиш.

Искаш да я подредиш. Но фактурите отиват при счетоводството, и грешка в тях е грешка в данъците.

Идеята

Refactoring: формата се сменя, поведението не

Refactoring е промяна на формата на кода, при която поведението остава същото: същите входове дават същите изходи. Не е „пренаписване“. Не е и „оправих и една грешка, докато бях там“. Това са други промени, които се правят отделно.

Прави се на малки стъпки с имена. Всяка стъпка е толкова малка, че почти не може да се обърка, и след всяка пускаш тестовете (предпазната мрежа от предишния урок). Имената са от каталога на Мартин Фаулър и са едни и същи във всеки език и всяко IDE: Rename, Extract Method, Inline, Introduce Parameter Object, Replace Conditional with Polymorphism.

Първо мрежата. Четири теста заковават четирите вида фактура, които магазинът печата днес: обикновен клиент, златен клиент, фирма над и под 500 €.

class InvoiceTest {
    List<Item> coffee = List.of(new Item("Кафе на зърна", 12.50, 2), new Item("Чаша", 8.00, 1));
    List<Item> moreCoffee = List.of(new Item("Кафе на зърна", 12.50, 4));
    List<Item> flour = List.of(new Item("Брашно 25 кг", 30.00, 20));

    @Test
    void regular() {
        assertEquals("Фактура: Мария Петрова\nул. Шипка 5, 1504 София\nСума: 33.00 €\nОтстъпка: 0.00 €\nОбщо с ДДС: 39.60 €",
                Invoices.inv("Мария Петрова", "ул. Шипка 5", "София", "1504", coffee, "regular"));
    }

    @Test
    void gold() {
        assertEquals("Фактура: Иван Георгиев\nул. Васил Левски 12, 4000 Пловдив\nСума: 50.00 €\nОтстъпка: 2.50 €\nОбщо с ДДС: 57.00 €",
                Invoices.inv("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000", moreCoffee, "gold"));
    }

    @Test
    void companyFrom500() {
        assertEquals("Фактура: Пекарна Изгрев ООД\nбул. Княз Борис I 80, 9000 Варна\nСума: 600.00 €\nОтстъпка: 60.00 €\nОбщо с ДДС: 648.00 €",
                Invoices.inv("Пекарна Изгрев ООД", "бул. Княз Борис I 80", "Варна", "9000", flour, "company"));
    }

    @Test
    void companyUnder500() {
        assertEquals("Фактура: Пекарна Изгрев ООД\nбул. Княз Борис I 80, 9000 Варна\nСума: 33.00 €\nОтстъпка: 0.00 €\nОбщо с ДДС: 39.60 €",
                Invoices.inv("Пекарна Изгрев ООД", "бул. Княз Борис I 80", "Варна", "9000", coffee, "company"));
    }
}

Резултат

ОК    companyFrom500()
ОК    companyUnder500()
ОК    gold()
ОК    regular()
Тестове: 4, минали: 4, паднали: 0

Виж го

Стъпка 1: Rename

Имената от една буква стават думи: t е subtotal, d е discount, s е text, inv е invoice. zip става postcode, а type става customerType.

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = 0;
    for (Item item : items) {
        subtotal = subtotal + item.price() * item.qty();
    }
    double discount = 0;
    if (customerType.equals("gold")) {
        discount = subtotal * 0.05;
    } else if (customerType.equals("company")) {
        if (subtotal >= 500) {
            discount = subtotal * 0.10;
        }
    }
    String text = "Фактура: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Сума: %.2f €\n", subtotal);
    text = text + String.format("Отстъпка: %.2f €\n", discount);
    text = text + String.format("Общо с ДДС: %.2f €", afterDiscount(subtotal, discount) * 1.20);
    return text;
}

Тестовете

ОК    companyFrom500()
ОК    companyUnder500()
ОК    gold()
ОК    regular()
Тестове: 4, минали: 4, паднали: 0

Rename сменя и всяко място, което вика метода, включително тестовете. Очакваните фактури не се пипат.

В IntelliJ IDEA е Shift+F6: IDE-то намира всяко използване по типовете, не по текста, и сменя само него.

Виж го

Стъпка 2: Extract Method

В метода има две отделни работи, които нямат нищо общо с текста на фактурата: сумата на редовете и отстъпката. Всяка отива в своя метод с име, което казва какво връща. Това е Extract Method:

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = discountFor(customerType, subtotal);
    String text = "Фактура: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Сума: %.2f €\n", subtotal);
    text = text + String.format("Отстъпка: %.2f €\n", discount);
    text = text + String.format("Общо с ДДС: %.2f €", afterDiscount(subtotal, discount) * 1.20);
    return text;
}

static double itemsTotal(List<Item> items) {
    double sum = 0;
    for (Item item : items) {
        sum = sum + item.price() * item.qty();
    }
    return sum;
}

static double discountFor(String customerType, double subtotal) {
    double discount = 0;
    if (customerType.equals("gold")) {
        discount = subtotal * 0.05;
    } else if (customerType.equals("company")) {
        if (subtotal >= 500) {
            discount = subtotal * 0.10;
        }
    }
    return discount;
}

Тестовете

ОК    companyFrom500()
ОК    companyUnder500()
ОК    gold()
ОК    regular()
Тестове: 4, минали: 4, паднали: 0

Сега invoice се чете отгоре надолу като списък: колко е, колко е отстъпката, какъв е текстът. Подробностите са един скок надолу, само ако ти трябват.

В IntelliJ IDEA: маркираш редовете и Ctrl+Alt+M (⌘⌥M на Mac).

IDE-то гледа кои променливи влизат в маркираните редове и кои излизат, и прави параметрите и върнатата стойност. Ако правиш стъпката на ръка, точно там се бърка.

Пробвай

Предвиди изхода

Някой прави Extract Method на ръка. Отстъпката отива в метод, който я записва в свой параметър, вместо да я върне:

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = 0;
    applyDiscount(customerType, subtotal, discount);
    String text = "Фактура: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Сума: %.2f €\n", subtotal);
    text = text + String.format("Отстъпка: %.2f €\n", discount);
    text = text + String.format("Общо с ДДС: %.2f €", afterDiscount(subtotal, discount) * 1.20);
    return text;
}

static void applyDiscount(String customerType, double subtotal, double discount) {
    if (customerType.equals("gold")) {
        discount = subtotal * 0.05;
    } else if (customerType.equals("company")) {
        if (subtotal >= 500) {
            discount = subtotal * 0.10;
        }
    }
}

Отговор

Падат gold() и companyFrom500()

Минават и четирите

Пада само companyFrom500()

Провери

По-точно, за любопитните

Резултат

ОК    companyUnder500()
ОК    regular()
ПАДА  companyFrom500()
ПАДА  gold()
Тестове: 4, минали: 2, паднали: 2

discount вътре в applyDiscount е копие на стойността. Новата стойност остава там и изчезва, когато методът свърши, а отвън discount е пак 0. Където отстъпката и без това е 0 (обикновен клиент, фирма под 500 €), тестовете минават. Затова мрежата има тест за всеки клон на if. Извлеченото трябва да върне стойността, както прави IDE-то.

Виж го

Стъпка 3: Inline

Обратното на Extract Method. afterDiscount прави едно изваждане и името му не казва нищо повече от subtotal - discount. Ползва се на едно място. Тялото отива там, където е извикването, а методът изчезва. Това е Inline:

static double afterDiscount(double subtotal, double discount) {
    return subtotal - discount;
}

static String invoice(String name, String street, String city, String postcode, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = discountFor(customerType, subtotal);
    String text = "Фактура: " + name + "\n";
    text = text + street + ", " + postcode + " " + city + "\n";
    text = text + String.format("Сума: %.2f €\n", subtotal);
    text = text + String.format("Отстъпка: %.2f €\n", discount);
    text = text + String.format("Общо с ДДС: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Тестовете

ОК    companyFrom500()
ОК    companyUnder500()
ОК    gold()
ОК    regular()
Тестове: 4, минали: 4, паднали: 0

Extract Method и Inline не си противоречат. Изваждаш, когато името обяснява нещо, което кодът не казва (itemsTotal). Вкарваш обратно, когато името е само скок (afterDiscount). В IntelliJ IDEA е Ctrl+Alt+N.

Виж го

Стъпка 4: Introduce Parameter Object

Името, улицата, градът и пощенският код вървят винаги заедно, а са четири параметъра подред. Размяната не се вижда:

Invoices.invoice("Мария Петрова", "ул. Шипка 5", "1504", "София", coffee, "regular")

Изход

Фактура: Мария Петрова
ул. Шипка 5, София 1504
Сума: 33.00 €
Отстъпка: 0.00 €
Общо с ДДС: 39.60 €

Четирите стават един обект: клиентът. Това е Introduce Parameter Object.

record пише полетата, конструктора и методите за четене:

record Customer(String name, String street, String city, String postcode) {}

static String invoice(Customer customer, List<Item> items, String customerType) {
    double subtotal = itemsTotal(items);
    double discount = discountFor(customerType, subtotal);
    String text = "Фактура: " + customer.name() + "\n";
    text = text + customer.street() + ", " + customer.postcode() + " " + customer.city() + "\n";
    text = text + String.format("Сума: %.2f €\n", subtotal);
    text = text + String.format("Отстъпка: %.2f €\n", discount);
    text = text + String.format("Общо с ДДС: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Тестовете

ОК    companyFrom500()
ОК    companyUnder500()
ОК    gold()
ОК    regular()
Тестове: 4, минали: 4, паднали: 0

Извикванията в тестовете се сменят с формата, а очакваните фактури остават същите. Ето едно извикване през стъпките:

Invoices.inv("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000", moreCoffee, "gold")
Invoices.invoice("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000", moreCoffee, "gold")
Invoices.invoice(new Customer("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000"), moreCoffee, "gold")
Invoices.invoice(new Customer("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000"), moreCoffee, new Gold())

В Java размяната е още възможна в конструктора на record, но вече е на едно място, където може да има проверка (пощенският код е четири цифри). Java няма аргументи по име. IntelliJ IDEA има „Introduce Parameter Object“ в менюто Refactor.

Виж го

Стъпка 5: Replace Conditional with Polymorphism

Остава веригата от if по вида клиент в discountFor. Всеки нов вид е нов клон тук, а утре и на други места (имейлът, отчетът). Всеки вид става клас, който имплементира общ интерфейс, и знае своята отстъпка:

interface CustomerType {
    double discount(double subtotal);
}

class Regular implements CustomerType {
    @Override
    public double discount(double subtotal) {
        return 0;
    }
}

class Gold implements CustomerType {
    @Override
    public double discount(double subtotal) {
        return subtotal * 0.05;
    }
}

class Company implements CustomerType {
    @Override
    public double discount(double subtotal) {
        if (subtotal >= 500) {
            return subtotal * 0.10;
        }
        return 0;
    }
}

static String invoice(Customer customer, List<Item> items, CustomerType customerType) {
    double subtotal = itemsTotal(items);
    double discount = customerType.discount(subtotal);
    String text = "Фактура: " + customer.name() + "\n";
    text = text + customer.street() + ", " + customer.postcode() + " " + customer.city() + "\n";
    text = text + String.format("Сума: %.2f €\n", subtotal);
    text = text + String.format("Отстъпка: %.2f €\n", discount);
    text = text + String.format("Общо с ДДС: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Тестовете

ОК    companyFrom500()
ОК    companyUnder500()
ОК    gold()
ОК    regular()
Тестове: 4, минали: 4, паднали: 0

Това е Strategy от модула за шаблоните: правилото е отделен обект, а фактурата само пита. Ако видът на клиента се сменя по време на работа (става златен след десетата поръчка), същият ход води до State. Текстът "gold" от базата се превръща в обект на едно място, на входа, с таблица като в урока Strategy.

record Customer(String name, String street, String city, String postcode) {}

interface CustomerType {
    double discount(double subtotal);
}

static String invoice(Customer customer, List<Item> items, CustomerType customerType) {
    double subtotal = itemsTotal(items);
    double discount = customerType.discount(subtotal);
    String text = "Фактура: " + customer.name() + "\n";
    text = text + customer.street() + ", " + customer.postcode() + " " + customer.city() + "\n";
    text = text + String.format("Сума: %.2f €\n", subtotal);
    text = text + String.format("Отстъпка: %.2f €\n", discount);
    text = text + String.format("Общо с ДДС: %.2f €", (subtotal - discount) * 1.20);
    return text;
}

Стъпка по стъпка, с думи

1. Функцията преди първата стъпка: 18 реда в едно. Долу са четирите теста и сумите на четирите фактури.
2. Rename: t става subtotal, d става discount, s става text. Тестовете са зелени, сумите са същите.
3. Extract Method: цикълът отива в itemsTotal, веригата от if в discountFor. На тяхно място остава по един ред.
4. Inline: afterDiscount само изваждаше. Тялото му се връща в реда, а функцията изчезва.
5. Introduce Parameter Object: име, улица, град и пощенски код стават един клиент.
6. Replace Conditional with Polymorphism: if-ът изчезва. Всеки вид клиент е клас зад CustomerType, а функцията вика customerType.discount(subtotal).
7. Пет стъпки. Формата на invoice е друга, а четирите теста и четирите фактури са същите като в началото.

Програмата, която печата четирите фактури, е пусната с кода отпреди първата стъпка и с кода след петата. Изходът е един и същ:

Изход преди и след

Фактура: Мария Петрова
ул. Шипка 5, 1504 София
Сума: 33.00 €
Отстъпка: 0.00 €
Общо с ДДС: 39.60 €

Фактура: Иван Георгиев
ул. Васил Левски 12, 4000 Пловдив
Сума: 50.00 €
Отстъпка: 2.50 €
Общо с ДДС: 57.00 €

Фактура: Пекарна Изгрев ООД
бул. Княз Борис I 80, 9000 Варна
Сума: 600.00 €
Отстъпка: 60.00 €
Общо с ДДС: 648.00 €

Фактура: Пекарна Изгрев ООД
бул. Княз Борис I 80, 9000 Варна
Сума: 33.00 €
Отстъпка: 0.00 €
Общо с ДДС: 39.60 €

Пробвай

Намери грешката

Някой пише класа за фирма така:

class Company implements CustomerType {
    @Override
    public double discount(double subtotal) {
        return subtotal * 0.10;
    }
}

Фирма поръчва кафе за 33 €:

Invoices.invoice(new Customer("Пекарна Изгрев ООД", "бул. Княз Борис I 80", "Варна", "9000"), coffee, new Company())

Изход

Фактура: Пекарна Изгрев ООД
бул. Княз Борис I 80, 9000 Варна
Сума: 33.00 €
Отстъпка: 3.30 €
Общо с ДДС: 35.64 €

На кой ред е грешката?

Избери реда с грешката.

1class Company implements CustomerType {2 @Override3 public double discount(double subtotal) {4 return subtotal * 0.10;5 }6}

Провери

По-точно, за любопитните

Тестовете

ОК    companyFrom500()
ОК    gold()
ОК    regular()
ПАДА  companyUnder500()
Тестове: 4, минали: 3, паднали: 1

Идеята

Кога да спреш

Refactoring няма край, ако не му сложиш. Спираш, когато кодът е достатъчно ясен за промяната, която идва. Задачата е да добавиш партньори с 3%? Стъпка 5 я прави един нов клас. Задачата е друга? Може би стъпка 5 не ти трябва.

Не си струва, когато:

• кодът не се пипа и никой не го чете. Грозният код, който работи и не се сменя, не е спешен;
• вариантите са два и няма да растат. Един if е по-ясен от интерфейс и два класа;
• под кода няма мрежа. Тогава първата стъпка е мрежата, не refactoring.

И едно правило: не смесвай. Или сменяш формата, или поведението, никога и двете в една стъпка. Кент Бек казва „две шапки“: с едната пренареждаш и тестовете не се пипат, с другата добавяш и пишеш нов тест. Сменяш шапката съзнателно.

Упражнение

Партньори с 3%

Реши задачата в редактора си.

Партньори. Магазинът добавя вид клиент „партньор“ с 3% отстъпка за всяка поръчка. Започни от кода след стъпка 5. Първо напиши тест за партньор с поръчката за 33 €, после класа Partner. Другите класове и invoice не се пипат.

Изход

Фактура: Мария Петрова
ул. Шипка 5, 1504 София
Сума: 33.00 €
Отстъпка: 0.99 €
Общо с ДДС: 38.41 €

Още задачи

• Extract Method за текста. Изведи реда с адреса в метод addressLine(Customer). Пускай тестовете след стъпката.
• Обратният ред. Направи петте стъпки в друг ред: първо Introduce Parameter Object, после Rename. Минават ли тестовете след всяка? Коя стъпка беше по-лесна по-рано?
• Видът от базата. Видът клиент идва като текст ("gold"). Напиши таблицата, която го превръща в обект, и реши какво става с непознат вид. Днешният код дава отстъпка 0 за непознат вид: запази това с тест.

Тест

Провери се

1. Кое от тези е refactoring?

Фактурата започва да показва и ДДС отделноЦикълът за сумата отива в отделен метод със същия резултатОтстъпката за златен клиент става 7%

2. След Introduce Parameter Object тестовете не се компилират, защото викат метода с шест параметъра. Какво правиш?

Сменяш и очакваните фактури, за да съвпаднатСменяш само извикванията в тестовете; очакваните фактури не се пипатИзтриваш тестовете и пишеш нови след края

3. Кога Inline е по-добро от Extract Method?

Когато методът е дълъгКогато малкият метод казва толкова, колкото и тялото му, и само добавя скок при четенеНикога: повече методи е винаги по-добре

4. Веригата от if по вида клиент става интерфейс и по клас за всеки вид. Как се казва това в модула за шаблоните?

SingletonObserverStrategy (или State, ако видът се сменя по време на работа)

5. Пет стъпки, тестовете след всяка. На четвъртата пада един тест. Какво значи това?

Тестът е грешенТрябва да се върнеш в началотоЧетвъртата стъпка е сменила поведението, а първите три са наред

Провери

Започни урока отначало

Свързани уроци

• Нужно отпреди: Имена в кода · Функции: една работа, малко параметри, без скрити странични ефекти · Миризмите: дълъг метод, голям клас, дублиране, примитиви вместо типове, дълга верига if · Тестовете като предпазна мрежа
• Продължава в: SOLID без догма · Упражнение: разхвърлян клас, изчистен стъпка по стъпка
• Задачи по темата: Чист код и refactoring