Функции: една работа, малко параметри, без скрити странични ефекти

Код на:JavaC#PythonJavaScript

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

Проблем

Функцията, която прави всичко

Имената от миналия урок вече са на място. Константите на магазина:

const decimal FreeDeliveryFrom = 50m;
const decimal StandardDeliveryFee = 4.99m;
const decimal ExpressSurcharge = 7m;
const string SpringCoupon = "SPRING10";
const decimal SpringDiscount = 0.10m;

static List<decimal> orders = new List<decimal>();

А това е функцията, през която минава всяка поръчка:

static void ProcessOrder(string email, decimal[] prices, string coupon, bool express)
{
    decimal amount = 0;
    foreach (decimal price in prices)
    {
        amount += price;
    }
    if (coupon == SpringCoupon)
    {
        amount = amount * (1 - SpringDiscount);
    }
    decimal delivery = amount >= FreeDeliveryFrom ? 0 : StandardDeliveryFee;
    if (express)
    {
        delivery += ExpressSurcharge;
    }
    decimal total = amount + delivery;
    orders.Add(total);
    Console.WriteLine($"До {email}: поръчката е приета, {total:F2} €");
    Console.WriteLine($"[log] поръчка №{orders.Count} за {total:F2} €");
}

ProcessOrder("maria@example.com", new decimal[] { 24.50m, 18.00m }, "SPRING10", true);
ProcessOrder("ivan@example.com", new decimal[] { 64.00m }, "", false);

Изход

До maria@example.com: поръчката е приета, 50.24 €
[log] поръчка №1 за 50.24 €
До ivan@example.com: поръчката е приета, 64.00 €
[log] поръчка №2 за 64.00 €

Работи. Сега маркетингът иска купонът да не важи за доставката. Счетоводството иска таксата да се показва отделно в количката. Тестерът иска да провери доставката под и над 50 €, а за всяка проверка трябва да направи поръчка, тоест да я запише и да прати имейл. Всички пипат една и съща функция. А какво значи true в края на извикването, пак не се вижда.

Идеята

Една работа и едно ниво

Опитай да кажеш какво прави ProcessOrder: „събира цените, и прилага купона, и смята доставката, и записва, и праща имейл, и пише в лога“. Всяко „и“ е отделна работа. Всяка от тях се сменя по своя причина и от различен човек.

Правилото е една работа на функция. Функцията с една работа е малка, има име, което казва цялата работа, и малко параметри. Как разбираш, че работата е една:

• описваш я с едно изречение без „и“;
• от нея не може да се извади парче с име, което не е просто преразказ на кода.

Второто правило е едно ниво на абстракция. Горната функция се чете като съдържание на книга: смятай сумата, запиши, потвърди, запиши в лога. Как точно се събират цените (циклите, сравненията, форматът на низа) е едно ниво по-долу, в малките функции. Когато в един и същ метод са „запиши поръчката“ и amount += price, читателят прескача между двете нива на всеки ред.

Виж го

Същата поръчка, разделена

Отгоре е съдържанието:

static void PlaceOrder(string email, decimal[] prices, string coupon, bool express)
{
    decimal total = OrderTotal(prices, coupon, express);
    orders.Add(total);
    SendConfirmation(email, total);
    LogOrder(orders.Count, total);
}

static void SendConfirmation(string email, decimal total)
{
    Console.WriteLine($"До {email}: поръчката е приета, {total:F2} €");
}

static void LogOrder(int number, decimal total)
{
    Console.WriteLine($"[log] поръчка №{number} за {total:F2} €");
}

Под него е сметката, всяка работа в своя функция:

static decimal Subtotal(decimal[] prices)
{
    decimal amount = 0;
    foreach (decimal price in prices)
    {
        amount += price;
    }
    return amount;
}

static decimal ApplyCoupon(decimal amount, string coupon)
{
    if (coupon == SpringCoupon)
    {
        return amount * (1 - SpringDiscount);
    }
    return amount;
}

static decimal DeliveryFee(decimal amount, bool express)
{
    decimal fee = amount >= FreeDeliveryFrom ? 0 : StandardDeliveryFee;
    if (express)
    {
        fee += ExpressSurcharge;
    }
    return fee;
}

static decimal OrderTotal(decimal[] prices, string coupon, bool express)
{
    decimal amount = Subtotal(prices);
    decimal discounted = ApplyCoupon(amount, coupon);
    return discounted + DeliveryFee(discounted, express);
}

PlaceOrder("maria@example.com", new decimal[] { 24.50m, 18.00m }, "SPRING10", true);
PlaceOrder("ivan@example.com", new decimal[] { 64.00m }, "", false);

Изход

До maria@example.com: поръчката е приета, 50.24 €
[log] поръчка №1 за 50.24 €
До ivan@example.com: поръчката е приета, 64.00 €
[log] поръчка №2 за 64.00 €

Изходът е същият. Сега купонът е в една функция от три реда, доставката в друга, и всяка се сменя, без да се пипа имейлът. Кодът е повече на брой редове, но всяко парче се чете, сменя и проверява отделно.

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

Колко малка? Няма число. Функция от 30 реда, която прави едно нещо (например дълъг, но равен switch с еднакви клонове), е наред. Функция от 8 реда с три нива на влагане и два флага не е. Мерилото е описанието с едно изречение, не броят на редовете. Разделянето на дълга функция на стъпки, така че програмата да работи след всяка, е Extract Method в шестия урок от модула.

Пробвай

Подреди редовете

Подреди OrderTotal така, че купонът да се приложи преди доставката. Защо редът има значение: отстъпката може да свали поръчката под 50 € и тогава доставката вече не е безплатна.

1. decimal amount = Subtotal(prices);↑↓
2. decimal discounted = ApplyCoupon(amount, coupon);↑↓
3. return discounted + DeliveryFee(discounted, express);↑↓
4. static decimal OrderTotal(decimal[] prices, string coupon, bool express)↑↓

Провери

Идеята

Малко параметри и без флагове

Всеки параметър е нещо, което читателят трябва да държи в главата си, и нещо, което всяко извикване трябва да подаде в правилния ред. Нула, един и два параметъра се четат лесно. Три вече е много. Четири и повече обикновено значат, че параметрите са едно нещо, разпиляно на части: имейлът, цените, купонът и доставката заедно са поръчка. Тогава те стават един обект. Това е refactoring-ът Introduce Parameter Object от шестия урок.

Най-лошият параметър е флагът: булева стойност, която избира какво да прави функцията.

static void PrintSlip(decimal[] prices, bool forCourier)
{
    if (forCourier)
    {
        Console.WriteLine($"Пратка: {prices.Length} бр.");
    }
    else
    {
        foreach (decimal price in prices)
        {
            Console.WriteLine($"{price:F2} €");
        }
    }
}

decimal[] cart = new decimal[] { 24.50m, 18.00m };
PrintSlip(cart, true);
PrintSlip(cart, false);

Изход

Пратка: 2 бр.
24.50 €
18.00 €

При извикването true не казва нищо. А вътре функцията има два отделни пътя, тоест две работи. Поправката е две функции с имена:

static void PrintCourierSlip(decimal[] prices)
{
    Console.WriteLine($"Пратка: {prices.Length} бр.");
}

static void PrintReceipt(decimal[] prices)
{
    foreach (decimal price in prices)
    {
        Console.WriteLine($"{price:F2} €");
    }
}

decimal[] cart = new decimal[] { 24.50m, 18.00m };
PrintCourierSlip(cart);
PrintReceipt(cart);

Изход

Пратка: 2 бр.
24.50 €
18.00 €

Понякога булевата стойност наистина е данни, а не избор на поведение: експресната доставка е част от поръчката. Тогава поне извикването трябва да казва какво значи:

C# има именувани аргументи: при извикването пишеш името на параметъра пред стойността.

Console.WriteLine($"{DeliveryFee(38.25m, express: true):F2} €");

Изход

11.99 €

Идеята

Страничен ефект

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

Страничните ефекти не са лоши сами по себе си. Поръчката трябва да се запише някъде, а имейлът трябва да тръгне. Лошо е, когато ефектът е скрит: името казва „питам“, а функцията и прави. Който я вика, не очаква нищо да се смени и я вика спокойно два пъти.

Виж го

Въпросът, който сменя отговора

Складът казва дали има лампи. Някой е решил, че щом има, е удобно веднага да ги запази:

class Warehouse
{
    public int Stock { get; private set; }

    public Warehouse(int stock)
    {
        Stock = stock;
    }

    public bool IsInStock(int quantity)
    {
        if (Stock >= quantity)
        {
            Stock -= quantity;
            return true;
        }
        return false;
    }
}

Количката пита, за да покаже „има“. После плащането пита същото:

var lamps = new Warehouse(3);
Console.WriteLine("Количка: " + (lamps.IsInStock(2) ? "има" : "няма"));
Console.WriteLine("Плащане: " + (lamps.IsInStock(2) ? "има" : "няма"));
Console.WriteLine("На склад: " + lamps.Stock);

Изход

Количка: има
Плащане: няма
На склад: 1

class Warehouse
{
    public int Stock { get; private set; }

    public Warehouse(int stock)
    {
        Stock = stock;
    }

    public bool IsInStock(int quantity)
    {
        if (Stock >= quantity)
        {
            Stock -= quantity;
            return true;
        }
        return false;
    }
}

var lamps = new Warehouse(3);
Console.WriteLine("Количка: " + (lamps.IsInStock(2) ? "има" : "няма"));
Console.WriteLine("Плащане: " + (lamps.IsInStock(2) ? "има" : "няма"));
Console.WriteLine("На склад: " + lamps.Stock);

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

1. Складът има 3 лампи на рафта.
2. Количката иска само да покаже дали има 2. Тя пита IsInStock(2): въпрос, съдейки по името.
3. 3 >= 2: има достатъчно.
4. Но въпросът и взима лампите: Stock става 1 от 3. Никой нищо не е купил, а количката не знае, че това е станало.
5. Количката получава true и показва „има“.
6. Клиентът отива към плащане. Плащането пита същото: IsInStock(2).
7. 1 >= 2 е false. Същият въпрос получава друг отговор, а клиентът вижда „няма“.
8. На склад: 1. Отговорът се смени, защото първият въпрос имаше скрит страничен ефект.

Никой нищо не е купил, а две лампи липсват, и клиентът не може да плати. В истински магазин това е поръчка, загубена заради страница, която пита за наличност. А грешката е трудна за намиране, защото всеки ред поотделно изглежда наред.

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

Още по-коварно е в свойство: lamps.Stock изглежда като четене на поле. Visual Studio изчислява свойствата, за да ги покаже в дебъгера, затова свойство със страничен ефект сменя състоянието само защото си спрял на точка за прекъсване и си погледнал обекта.

Пробвай

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

Складът с IsInStock, който и запазва, вече има 5 лампи. Един и същ въпрос се задава три пъти. Какво печата програмата?

var lamps = new Warehouse(5);
Console.WriteLine(lamps.IsInStock(2));
Console.WriteLine(lamps.IsInStock(2));
Console.WriteLine(lamps.IsInStock(2));

Отговор

True / False / False

True / True / False

True / True / True

Провери

Идеята

Команда или въпрос, не и двете

Правилото се казва разделяне на команда и въпрос (command–query separation): функцията или отговаря на въпрос и не сменя нищо, или прави нещо и не се преструва на въпрос. Складът става така:

class Warehouse
{
    public int Stock { get; private set; }

    public Warehouse(int stock)
    {
        Stock = stock;
    }

    public bool IsInStock(int quantity)
    {
        return Stock >= quantity;
    }

    public void Reserve(int quantity)
    {
        if (!IsInStock(quantity))
        {
            throw new InvalidOperationException("Няма достатъчно наличност");
        }
        Stock -= quantity;
    }
}

var lamps = new Warehouse(3);
Console.WriteLine("Количка: " + (lamps.IsInStock(2) ? "има" : "няма"));
Console.WriteLine("Плащане: " + (lamps.IsInStock(2) ? "има" : "няма"));
lamps.Reserve(2);
Console.WriteLine("На склад: " + lamps.Stock);

Изход

Количка: има
Плащане: има
На склад: 1

Количката и плащането питат колкото искат. Лампите се запазват на един ред, който казва точно това: Reserve(2).

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

Има известни изключения, и името ги издава: stack.Pop() маха елемента и го връща, queue.Dequeue() мести напред и връща. Това са глаголи, не въпроси, затова никой не ги вика „просто да провери“. Правилото не е „командата никога не връща нищо“, а „въпросът никога не сменя нищо“.

Идеята

Чистата функция

DeliveryFee от разделената поръчка отговаря само по параметрите си и не сменя нищо. Такава функция се казва чиста: при едни и същи параметри винаги връща едно и също, колкото и пъти и в какъвто и ред да я викаш.

Console.WriteLine($"{DeliveryFee(38.25m, false):F2} €");
Console.WriteLine($"{DeliveryFee(38.25m, true):F2} €");
Console.WriteLine($"{DeliveryFee(64.00m, false):F2} €");
Console.WriteLine($"{DeliveryFee(38.25m, true):F2} €");

Изход

4.99 €
11.99 €
0.00 €
11.99 €

Проверката на доставката под и над прага е четири реда. Няма склад, който да се напълни, няма имейл, който да тръгне, няма база. При ProcessOrder за същото трябваше да направиш поръчка. Затова сметките се пазят чисти, а страничните ефекти се събират в няколко ясно кръстени команди отгоре. Как такива проверки стават тестове, които пазят кода при всяка промяна, е темата на петия урок от модула.

Пробвай

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

Бутонът „Махни последното“ трябва да маха продукта, който клиентът е добавил последен. Клиентът добавя мляко, ябълки и хляб. Преди това магазинът печата бележка, подредена по азбучен ред:

static void PrintReceipt(List<string> items)
{
    items.Sort();
    foreach (string item in items)
    {
        Console.WriteLine("  " + item);
    }
}

var cart = new List<string> { "мляко", "ябълки", "хляб" };
PrintReceipt(cart);
Console.WriteLine("Последно добавено: " + cart[cart.Count - 1]);

Изход

  мляко
  хляб
  ябълки
Последно добавено: ябълки

Последен е добавен хлябът, а програмата казва ябълки. На кой ред е грешката?

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

1static void PrintReceipt(List<string> items)2{3 items.Sort();4 foreach (string item in items)5 {6 Console.WriteLine(" " + item);7 }8}

Провери

Упражнение

Известието за изпратена поръчка

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

Известието. Тази функция смята текста, избира канала с флаг, печата и брои изпратените:

static int sent = 0;

static void NotifyShipped(string name, int orderId, bool sms)
{
    sent++;
    string text = $"Здравей, {name}! Поръчка {orderId} е изпратена.";
    if (sms)
    {
        Console.WriteLine("SMS: " + text);
    }
    else
    {
        Console.WriteLine("Имейл: " + text);
    }
}

NotifyShipped("Мария", 1042, false);
NotifyShipped("Иван", 1043, true);
Console.WriteLine("Изпратени: " + sent);

Изход

Имейл: Здравей, Мария! Поръчка 1042 е изпратена.
SMS: Здравей, Иван! Поръчка 1043 е изпратена.
Изпратени: 2

Раздели я на три функции: чиста, която само прави текста, и две команди, едната праща по имейл, другата по SMS, и всяка брои изпратеното. Флагът изчезва. Изходът трябва да остане същият:

Изход

Имейл: Здравей, Мария! Поръчка 1042 е изпратена.
SMS: Здравей, Иван! Поръчка 1043 е изпратена.
Изпратени: 2

Още задачи

• Параметър обект. PlaceOrder има четири параметъра. Събери ги в клас Order и виж как изглежда извикването.
• Купонът не важи за доставката. Маркетингът сменя правилото. Коя функция пипаш и колко други остават непокътнати?
• Опашката. Напиши PeekNext() и TakeNext() за опашка от поръчки за куриера. Кое е въпрос и кое е команда?

Тест

Провери се

1. Как разбираш, че ProcessOrder прави повече от една работа?

По това, че има цикълОписанието ѝ има много „и“: събира и прилага купона и смята доставката и записва и праща имейлПо броя на редовете

2. PrintSlip(cart, true). Какво не е наред?

Параметрите трябва да са повечеФлагът казва, че функцията прави две различни неща. По-ясно е PrintCourierSlip(cart) и PrintReceipt(cart)Нищо

3. IsInStock(2) намалява наличността, ако има достатъчно. Количката пита веднъж, плащането още веднъж. Какво става при 3 лампи?

Количката получава „има“, плащането „няма“, а на склад остава 1И двете получават „има“Програмата спира с грешка

4. Коя от тези функции е чиста?

DeliveryFee(amount, express): смята таксата от двата параметъраPrintReceipt(items): подрежда списъка, който получава, и го печатаPlaceOrder(…): записва поръчката и праща имейл

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

Всяка функция трябва да връща стойностКомандите трябва да са в отделен класФункцията или прави нещо, или отговаря на въпрос, но не и двете

Провери

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

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

• Нужно отпреди: Изключения · Хвърляне и свои изключения · Списъци · Имена в кода
• Продължава в: Коментари, които лъжат, и такива, които помагат · Миризмите: дълъг метод, голям клас, дублиране, примитиви вместо типове, дълга верига if · Тестовете като предпазна мрежа · Refactoring на малки стъпки
• Задачи по темата: Чист код и refactoring