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

Код на:JavaC#PythonJavaScript

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

Проблем

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

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

export function inv(n, street, city, zip, items, type) {
  let t = 0;
  for (const i of items) {
    t = t + i.price * i.qty;
  }
  let d = 0;
  if (type === "gold") {
    d = t * 0.05;
  } else if (type === "company") {
    if (t >= 500) {
      d = t * 0.1;
    }
  }
  let s = `Фактура: ${n}\n`;
  s = s + `${street}, ${zip} ${city}\n`;
  s = s + `Сума: ${t.toFixed(2)} €\n`;
  s = s + `Отстъпка: ${d.toFixed(2)} €\n`;
  s = s + `Общо с ДДС: ${(afterDiscount(t, d) * 1.2).toFixed(2)} €`;
  return s;
}

function afterDiscount(t, 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 €.

import { test } from "node:test";
import assert from "node:assert/strict";
import { inv } from "./invoice.js";

const coffee = [{ name: "Кафе на зърна", price: 12.5, qty: 2 }, { name: "Чаша", price: 8, qty: 1 }];
const moreCoffee = [{ name: "Кафе на зърна", price: 12.5, qty: 4 }];
const flour = [{ name: "Брашно 25 кг", price: 30, qty: 20 }];

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

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

test("фирма от 500 €", () => {
  assert.equal(
    inv("Пекарна Изгрев ООД", "бул. Княз Борис I 80", "Варна", "9000", flour, "company"),
    "Фактура: Пекарна Изгрев ООД\nбул. Княз Борис I 80, 9000 Варна\nСума: 600.00 €\nОтстъпка: 60.00 €\nОбщо с ДДС: 648.00 €",
  );
});

test("фирма под 500 €", () => {
  assert.equal(
    inv("Пекарна Изгрев ООД", "бул. Княз Борис I 80", "Варна", "9000", coffee, "company"),
    "Фактура: Пекарна Изгрев ООД\nбул. Княз Борис I 80, 9000 Варна\nСума: 33.00 €\nОтстъпка: 0.00 €\nОбщо с ДДС: 39.60 €",
  );
});

Резултат

ОК    златен клиент
ОК    обикновен клиент
ОК    фирма от 500 €
ОК    фирма под 500 €
Тестове: 4, минали: 4, паднали: 0

Виж го

Стъпка 1: Rename

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

export function invoice(name, street, city, postcode, items, customerType) {
  let subtotal = 0;
  for (const item of items) {
    subtotal = subtotal + item.price * item.qty;
  }
  let discount = 0;
  if (customerType === "gold") {
    discount = subtotal * 0.05;
  } else if (customerType === "company") {
    if (subtotal >= 500) {
      discount = subtotal * 0.1;
    }
  }
  let text = `Фактура: ${name}\n`;
  text = text + `${street}, ${postcode} ${city}\n`;
  text = text + `Сума: ${subtotal.toFixed(2)} €\n`;
  text = text + `Отстъпка: ${discount.toFixed(2)} €\n`;
  text = text + `Общо с ДДС: ${(afterDiscount(subtotal, discount) * 1.2).toFixed(2)} €`;
  return text;
}

Тестовете

ОК    златен клиент
ОК    обикновен клиент
ОК    фирма от 500 €
ОК    фирма под 500 €
Тестове: 4, минали: 4, паднали: 0

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

Във VS Code е F2. За локална променлива е сигурно. За свойство на обект (customer.postcode) редакторът не знае кой обект е кой: obj[key] или обект, дошъл като JSON, не са връзка, която той вижда. Затова в JavaScript след преименуване тестовете казват повече, отколкото в Java или C#.

Виж го

Стъпка 2: Extract Method

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

export function invoice(name, street, city, postcode, items, customerType) {
  const subtotal = itemsTotal(items);
  const discount = discountFor(customerType, subtotal);
  let text = `Фактура: ${name}\n`;
  text = text + `${street}, ${postcode} ${city}\n`;
  text = text + `Сума: ${subtotal.toFixed(2)} €\n`;
  text = text + `Отстъпка: ${discount.toFixed(2)} €\n`;
  text = text + `Общо с ДДС: ${(afterDiscount(subtotal, discount) * 1.2).toFixed(2)} €`;
  return text;
}

function itemsTotal(items) {
  let sum = 0;
  for (const item of items) {
    sum = sum + item.price * item.qty;
  }
  return sum;
}

function discountFor(customerType, subtotal) {
  let discount = 0;
  if (customerType === "gold") {
    discount = subtotal * 0.05;
  } else if (customerType === "company") {
    if (subtotal >= 500) {
      discount = subtotal * 0.1;
    }
  }
  return discount;
}

Тестовете

ОК    златен клиент
ОК    обикновен клиент
ОК    фирма от 500 €
ОК    фирма под 500 €
Тестове: 4, минали: 4, паднали: 0

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

Във VS Code: маркираш редовете, Ctrl+. и „Extract to function in module scope“.

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

Пробвай

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

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

export function invoice(name, street, city, postcode, items, customerType) {
  const subtotal = itemsTotal(items);
  const discount = 0;
  applyDiscount(customerType, subtotal, discount);
  let text = `Фактура: ${name}\n`;
  text = text + `${street}, ${postcode} ${city}\n`;
  text = text + `Сума: ${subtotal.toFixed(2)} €\n`;
  text = text + `Отстъпка: ${discount.toFixed(2)} €\n`;
  text = text + `Общо с ДДС: ${(afterDiscount(subtotal, discount) * 1.2).toFixed(2)} €`;
  return text;
}

function applyDiscount(customerType, subtotal, discount) {
  if (customerType === "gold") {
    discount = subtotal * 0.05;
  } else if (customerType === "company") {
    if (subtotal >= 500) {
      discount = subtotal * 0.1;
    }
  }
}

Отговор

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

Падат „златен клиент“ и „фирма от 500 €“

Пада само „фирма от 500 €“

Провери

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

Резултат

ОК    обикновен клиент
ОК    фирма под 500 €
ПАДА  златен клиент
ПАДА  фирма от 500 €
Тестове: 4, минали: 2, паднали: 2

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

Виж го

Стъпка 3: Inline

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

function afterDiscount(subtotal, discount) {
  return subtotal - discount;
}

export function invoice(name, street, city, postcode, items, customerType) {
  const subtotal = itemsTotal(items);
  const discount = discountFor(customerType, subtotal);
  let text = `Фактура: ${name}\n`;
  text = text + `${street}, ${postcode} ${city}\n`;
  text = text + `Сума: ${subtotal.toFixed(2)} €\n`;
  text = text + `Отстъпка: ${discount.toFixed(2)} €\n`;
  text = text + `Общо с ДДС: ${((subtotal - discount) * 1.2).toFixed(2)} €`;
  return text;
}

Тестовете

ОК    златен клиент
ОК    обикновен клиент
ОК    фирма от 500 €
ОК    фирма под 500 €
Тестове: 4, минали: 4, паднали: 0

Extract Method и Inline не си противоречат. Изваждаш, когато името обяснява нещо, което кодът не казва (itemsTotal). Вкарваш обратно, когато името е само скок (afterDiscount). Във VS Code за променлива е Ctrl+. и „Inline variable“; функция се вкарва на ръка.

Виж го

Стъпка 4: Introduce Parameter Object

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

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

Изход

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

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

В JavaScript клас не трябва: обектът е обикновен обект с имена на полетата, { name, street, city, postcode }.

export function invoice(customer, items, customerType) {
  const subtotal = itemsTotal(items);
  const discount = discountFor(customerType, subtotal);
  let text = `Фактура: ${customer.name}\n`;
  text = text + `${customer.street}, ${customer.postcode} ${customer.city}\n`;
  text = text + `Сума: ${subtotal.toFixed(2)} €\n`;
  text = text + `Отстъпка: ${discount.toFixed(2)} €\n`;
  text = text + `Общо с ДДС: ${((subtotal - discount) * 1.2).toFixed(2)} €`;
  return text;
}

Тестовете

ОК    златен клиент
ОК    обикновен клиент
ОК    фирма от 500 €
ОК    фирма под 500 €
Тестове: 4, минали: 4, паднали: 0

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

inv("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000", moreCoffee, "gold")
invoice("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000", moreCoffee, "gold")
invoice({ name: "Иван Георгиев", street: "ул. Васил Левски 12", city: "Пловдив", postcode: "4000" }, moreCoffee, "gold")
invoice({ name: "Иван Георгиев", street: "ул. Васил Левски 12", city: "Пловдив", postcode: "4000" }, moreCoffee, new Gold())

В JavaScript обектът е с имена на полетата, и размяна на места няма: { city: "София", postcode: "1504" } е същото като { postcode: "1504", city: "София" }.

Виж го

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

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

export class Regular {
  discount(subtotal) {
    return 0;
  }
}

export class Gold {
  discount(subtotal) {
    return subtotal * 0.05;
  }
}

export class Company {
  discount(subtotal) {
    if (subtotal >= 500) {
      return subtotal * 0.1;
    }
    return 0;
  }
}

export function invoice(customer, items, customerType) {
  const subtotal = itemsTotal(items);
  const discount = customerType.discount(subtotal);
  let text = `Фактура: ${customer.name}\n`;
  text = text + `${customer.street}, ${customer.postcode} ${customer.city}\n`;
  text = text + `Сума: ${subtotal.toFixed(2)} €\n`;
  text = text + `Отстъпка: ${discount.toFixed(2)} €\n`;
  text = text + `Общо с ДДС: ${((subtotal - discount) * 1.2).toFixed(2)} €`;
  return text;
}

Тестовете

ОК    златен клиент
ОК    обикновен клиент
ОК    фирма от 500 €
ОК    фирма под 500 €
Тестове: 4, минали: 4, паднали: 0

Тук няма интерфейс: всеки клас просто има метод discount, и фактурата го вика (duck typing). Ако правилото е само едно смятане, функция в таблица стига, както в урока Strategy.

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

export function invoice(customer, items, customerType) {
  const subtotal = itemsTotal(items);
  const discount = customerType.discount(subtotal);
  let text = `Фактура: ${customer.name}\n`;
  text = text + `${customer.street}, ${customer.postcode} ${customer.city}\n`;
  text = text + `Сума: ${subtotal.toFixed(2)} €\n`;
  text = text + `Отстъпка: ${discount.toFixed(2)} €\n`;
  text = text + `Общо с ДДС: ${((subtotal - discount) * 1.2).toFixed(2)} €`;
  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.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 €

Пробвай

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

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

export class Company {
  discount(subtotal) {
    return subtotal * 0.1;
  }
}

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

invoice({ name: "Пекарна Изгрев ООД", street: "бул. Княз Борис I 80", city: "Варна", postcode: "9000" }, coffee, new Company())

Изход

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

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

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

1export class Company {2 discount(subtotal) {3 return subtotal * 0.1;4 }5}

Провери

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

Тестовете

ОК    златен клиент
ОК    обикновен клиент
ОК    фирма от 500 €
ПАДА  фирма под 500 €
Тестове: 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