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

Проблем

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

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

def inv(n, street, city, zip, items, type):
    t = 0
    for i in items:
        t = t + i.price * i.qty
    d = 0
    if type == "gold":
        d = t * 0.05
    elif type == "company":
        if t >= 500:
            d = t * 0.10
    s = "Фактура: " + n + "\n"
    s = s + street + ", " + zip + " " + city + "\n"
    s = s + f"Сума: {t:.2f} €\n"
    s = s + f"Отстъпка: {d:.2f} €\n"
    s = s + f"Общо с ДДС: {after_discount(t, d) * 1.20:.2f} €"
    return s

def after_discount(t, d):
    return t - d

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

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

Идеята

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

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

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

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

COFFEE = [Item("Кафе на зърна", 12.50, 2), Item("Чаша", 8.00, 1)]
MORE_COFFEE = [Item("Кафе на зърна", 12.50, 4)]
FLOUR = [Item("Брашно 25 кг", 30.00, 20)]


class InvoiceTest(unittest.TestCase):
    def test_regular(self):
        self.assertEqual(
            inv("Мария Петрова", "ул. Шипка 5", "София", "1504", COFFEE, "regular"),
            "Фактура: Мария Петрова\nул. Шипка 5, 1504 София\nСума: 33.00 €\nОтстъпка: 0.00 €\nОбщо с ДДС: 39.60 €",
        )

    def test_gold(self):
        self.assertEqual(
            inv("Иван Георгиев", "ул. Васил Левски 12", "Пловдив", "4000", MORE_COFFEE, "gold"),
            "Фактура: Иван Георгиев\nул. Васил Левски 12, 4000 Пловдив\nСума: 50.00 €\nОтстъпка: 2.50 €\nОбщо с ДДС: 57.00 €",
        )

    def test_company_from_500(self):
        self.assertEqual(
            inv("Пекарна Изгрев ООД", "бул. Княз Борис I 80", "Варна", "9000", FLOUR, "company"),
            "Фактура: Пекарна Изгрев ООД\nбул. Княз Борис I 80, 9000 Варна\nСума: 600.00 €\nОтстъпка: 60.00 €\nОбщо с ДДС: 648.00 €",
        )

    def test_company_under_500(self):
        self.assertEqual(
            inv("Пекарна Изгрев ООД", "бул. Княз Борис I 80", "Варна", "9000", COFFEE, "company"),
            "Фактура: Пекарна Изгрев ООД\nбул. Княз Борис I 80, 9000 Варна\nСума: 33.00 €\nОтстъпка: 0.00 €\nОбщо с ДДС: 39.60 €",
        )

ОК    test_company_from_500
ОК    test_company_under_500
ОК    test_gold
ОК    test_regular
Тестове: 4, минали: 4, паднали: 0

Виж го

Стъпка 1: Rename

Имената от една буква стават думи: t е subtotal, d е discount, s е text, inv е invoice. zip и type са и имена на вградени функции в Python, които параметрите скриват. Стават postcode и customer_type.

def invoice(name, street, city, postcode, items, customer_type):
    subtotal = 0
    for item in items:
        subtotal = subtotal + item.price * item.qty
    discount = 0
    if customer_type == "gold":
        discount = subtotal * 0.05
    elif customer_type == "company":
        if subtotal >= 500:
            discount = subtotal * 0.10
    text = "Фактура: " + name + "\n"
    text = text + street + ", " + postcode + " " + city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {after_discount(subtotal, discount) * 1.20:.2f} €"
    return text

ОК    test_company_from_500
ОК    test_company_under_500
ОК    test_gold
ОК    test_regular
Тестове: 4, минали: 4, паднали: 0

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

В PyCharm е Shift+F6, във VS Code е F2. Без типове обаче IDE-то понякога гадае: име в низ, getattr(obj, "name") или ключ на речник не са връзка, която то вижда. Затова в Python след преименуване тестовете казват повече, отколкото в Java или C#.

Виж го

Стъпка 2: Extract Method

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

def invoice(name, street, city, postcode, items, customer_type):
    subtotal = items_total(items)
    discount = discount_for(customer_type, subtotal)
    text = "Фактура: " + name + "\n"
    text = text + street + ", " + postcode + " " + city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {after_discount(subtotal, discount) * 1.20:.2f} €"
    return text

def items_total(items):
    total = 0
    for item in items:
        total = total + item.price * item.qty
    return total


def discount_for(customer_type, subtotal):
    discount = 0
    if customer_type == "gold":
        discount = subtotal * 0.05
    elif customer_type == "company":
        if subtotal >= 500:
            discount = subtotal * 0.10
    return discount

ОК    test_company_from_500
ОК    test_company_under_500
ОК    test_gold
ОК    test_regular
Тестове: 4, минали: 4, паднали: 0

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

В PyCharm: маркираш редовете и Ctrl+Alt+M. Във VS Code: Ctrl+. и „Extract method“.

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

Пробвай

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

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

def invoice(name, street, city, postcode, items, customer_type):
    subtotal = items_total(items)
    discount = 0
    apply_discount(customer_type, subtotal, discount)
    text = "Фактура: " + name + "\n"
    text = text + street + ", " + postcode + " " + city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {after_discount(subtotal, discount) * 1.20:.2f} €"
    return text


def apply_discount(customer_type, subtotal, discount):
    if customer_type == "gold":
        discount = subtotal * 0.05
    elif customer_type == "company":
        if subtotal >= 500:
            discount = subtotal * 0.10

Отговор

Падат test_gold и test_company_from_500

Пада само test_company_from_500

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

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

ОК    test_company_under_500
ОК    test_regular
ПАДА  test_company_from_500
ПАДА  test_gold
Тестове: 4, минали: 2, паднали: 2

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

Виж го

Стъпка 3: Inline

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

def after_discount(subtotal, discount):
    return subtotal - discount

def invoice(name, street, city, postcode, items, customer_type):
    subtotal = items_total(items)
    discount = discount_for(customer_type, subtotal)
    text = "Фактура: " + name + "\n"
    text = text + street + ", " + postcode + " " + city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {(subtotal - discount) * 1.20:.2f} €"
    return text

ОК    test_company_from_500
ОК    test_company_under_500
ОК    test_gold
ОК    test_regular
Тестове: 4, минали: 4, паднали: 0

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

Виж го

Стъпка 4: Introduce Parameter Object

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

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

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

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

@dataclass пише __init__ и сравнението:

@dataclass(frozen=True)
class Customer:
    name: str
    street: str
    city: str
    postcode: str

def invoice(customer, items, customer_type):
    subtotal = items_total(items)
    discount = discount_for(customer_type, subtotal)
    text = "Фактура: " + customer.name + "\n"
    text = text + customer.street + ", " + customer.postcode + " " + customer.city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {(subtotal - discount) * 1.20:.2f} €"
    return text

ОК    test_company_from_500
ОК    test_company_under_500
ОК    test_gold
ОК    test_regular
Тестове: 4, минали: 4, паднали: 0

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

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

В Python и конструкторът на dataclass, и функциите приемат аргументи по име: Customer(name="...", city="...", ...). Редът тогава не е важен.

Виж го

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

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

class Regular:
    def discount(self, subtotal):
        return 0


class Gold:
    def discount(self, subtotal):
        return subtotal * 0.05


class Company:
    def discount(self, subtotal):
        if subtotal >= 500:
            return subtotal * 0.10
        return 0

def invoice(customer, items, customer_type):
    subtotal = items_total(items)
    discount = customer_type.discount(subtotal)
    text = "Фактура: " + customer.name + "\n"
    text = text + customer.street + ", " + customer.postcode + " " + customer.city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {(subtotal - discount) * 1.20:.2f} €"
    return text

ОК    test_company_from_500
ОК    test_company_under_500
ОК    test_gold
ОК    test_regular
Тестове: 4, минали: 4, паднали: 0

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

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

@dataclass(frozen=True)
class Customer:
    name: str
    street: str
    city: str
    postcode: str


def invoice(customer, items, customer_type):
    subtotal = items_total(items)
    discount = customer_type.discount(subtotal)
    text = "Фактура: " + customer.name + "\n"
    text = text + customer.street + ", " + customer.postcode + " " + customer.city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {(subtotal - discount) * 1.20:.2f} €"
    return text

def inv(n, street, city, zip, items, type):
    t = 0
    for i in items:
        t = t + i.price * i.qty
    d = 0
    if type == "gold":
        d = t * 0.05
    elif type == "company":
        if t >= 500:
            d = t * 0.10
    s = "Фактура: " + n + "\n"
    s = s + street + ", " + zip + " " + city + "\n"
    s = s + f"Сума: {t:.2f} €\n"
    s = s + f"Отстъпка: {d:.2f} €\n"
    s = s + f"Общо с ДДС: {after_discount(t, d) * 1.20:.2f} €"
    return s

def invoice(name, street, city, postcode, items, customer_type):
    subtotal = 0
    for item in items:
        subtotal = subtotal + item.price * item.qty
    discount = 0
    if customer_type == "gold":
        discount = subtotal * 0.05
    elif customer_type == "company":
        if subtotal >= 500:
            discount = subtotal * 0.10
    text = "Фактура: " + name + "\n"
    text = text + street + ", " + postcode + " " + city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {after_discount(subtotal, discount) * 1.20:.2f} €"
    return text

def invoice(name, street, city, postcode, items, customer_type):
    subtotal = items_total(items)
    discount = discount_for(customer_type, subtotal)
    text = "Фактура: " + name + "\n"
    text = text + street + ", " + postcode + " " + city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {after_discount(subtotal, discount) * 1.20:.2f} €"
    return text

def invoice(name, street, city, postcode, items, customer_type):
    subtotal = items_total(items)
    discount = discount_for(customer_type, subtotal)
    text = "Фактура: " + name + "\n"
    text = text + street + ", " + postcode + " " + city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {(subtotal - discount) * 1.20:.2f} €"
    return text

@dataclass(frozen=True)
class Customer:
    name: str
    street: str
    city: str
    postcode: str


def invoice(customer, items, customer_type):
    subtotal = items_total(items)
    discount = discount_for(customer_type, subtotal)
    text = "Фактура: " + customer.name + "\n"
    text = text + customer.street + ", " + customer.postcode + " " + customer.city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {(subtotal - discount) * 1.20:.2f} €"
    return text

@dataclass(frozen=True)
class Customer:
    name: str
    street: str
    city: str
    postcode: str


def invoice(customer, items, customer_type):
    subtotal = items_total(items)
    discount = customer_type.discount(subtotal)
    text = "Фактура: " + customer.name + "\n"
    text = text + customer.street + ", " + customer.postcode + " " + customer.city + "\n"
    text = text + f"Сума: {subtotal:.2f} €\n"
    text = text + f"Отстъпка: {discount:.2f} €\n"
    text = text + f"Общо с ДДС: {(subtotal - discount) * 1.20:.2f} €"
    return text

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

Функцията преди първата стъпка: 15 реда в едно. Долу са четирите теста и сумите на четирите фактури.
Rename: t става subtotal, d става discount, s става text. Тестовете са зелени, сумите са същите.
Extract Method: цикълът отива в items_total, веригата от if в discount_for. На тяхно място остава по един ред.
Inline: after_discount само изваждаше. Тялото му се връща в реда, а функцията изчезва.
Introduce Parameter Object: име, улица, град и пощенски код стават един клиент.
Replace Conditional with Polymorphism: if-ът изчезва. Всеки вид клиент е клас, а функцията вика customer_type.discount(subtotal).
Пет стъпки. Формата на 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:
    def discount(self, subtotal):
        return subtotal * 0.10

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

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

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

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

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

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

ОК    test_company_from_500
ОК    test_gold
ОК    test_regular
ПАДА  test_company_under_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 за текста. Изведи реда с адреса в функция address_line(customer). Пускай тестовете след стъпката.
Обратният ред. Направи петте стъпки в друг ред: първо Introduce Parameter Object, после Rename. Минават ли тестовете след всяка? Коя стъпка беше по-лесна по-рано?
Видът от базата. Видът клиент идва като текст ("gold"). Напиши таблицата, която го превръща в обект, и реши какво става с непознат вид. Днешният код дава отстъпка 0 за непознат вид: запази това с тест.

Тест

Провери се

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

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

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

Метод, който само вади отстъпката от сумата, не казва нищо повече от subtotal - discount. Методът си заслужава, когато името обяснява нещо, което тялото не казва.

4. Веригата от if по вида клиент става интерфейс и по клас за всеки вид. Как се казва това в модула за шаблоните?SingletonObserverStrategy (или State, ако видът се сменя по време на работа)

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

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

Бисквитки