Skip to content
wannacode

Exercise: a messy class, cleaned up step by step

In short: A refactoring exercise: a scooter rental class is cleaned up in five small steps (names, magic numbers, money, query apart from command, plans) while the tests stay green.

Problem

The scooters nobody dares to touch

A company that rents out e-scooters bills every ride with the RideMgr class. It has worked for three years. The Calc method works out the price from the customer's plan (pay as you go, student or monthly pass), drops the unlock fee from the fourth ride of the day, adds a night surcharge, adds up the revenue and sends a receipt:

public class RideMgr
{
    private readonly Dictionary<string, int> c = new();
    public double S { get; private set; }
    public List<string> Out { get; } = new();


    // Price: €1 to unlock and €0.15 per minute.
    // At night the price goes up by 50%.
    public double Calc(string to, string t, int m, bool f)
    {
        c[to] = c.GetValueOrDefault(to) + 1;
        double p = 0;
        if (t == "payg")
        {
            p = 1.0 + m * 0.2;
        }
        else if (t == "student")
        {
            p = (1.0 + m * 0.2) * 0.8;
        }
        else if (t == "monthly")
        {
            if (m > 45)
            {
                p = (m - 45) * 0.2;
            }
        }
        if (c[to] > 3 && t != "monthly")
        {
            p = p - 1.0;
        }
        if (f)
        {
            p = p + 0.5;
        }
        S = S + p;
        Out.Add($"{to}: {m} min, €{p:F2}");
        return p;
    }
}

Next month two changes arrive:

  • a business plan: no unlock fee, €0.18 per minute, no discount for the fourth ride;
  • the price before the end: the app shows what the ride will cost while it is still going.

The second one cannot be done with this class. Here is what happens if the app calls Calc to show the price, and then again to take the money:

var rides = new RideMgr();
double shown = rides.Calc("ana@example.com", "payg", 10, false);
Console.WriteLine($"Price before the end: €{shown:F2}");
double charged = rides.Calc("ana@example.com", "payg", 10, false);
Console.WriteLine($"Charged: €{charged:F2}");
Console.WriteLine($"Receipts: {rides.Out.Count}");
Console.WriteLine($"Revenue: €{rides.S:F2}");
Output
Price before the end: €3.00
Charged: €3.00
Receipts: 2
Revenue: €6.00

One ride, two receipts and €6 of revenue. Calc sounds like a question, but it counts, adds up and sends.

There is no new idea in this lesson. It is an exercise: the class goes through the tools of the module one by one, in small steps, and you run the tests after each step.

See it

Step 0: the safety net

First the net, as in the lesson on tests. Nine tests pin what the class does today: the price for each plan, the night, the fourth ride, the receipt and the revenue.

using Xunit;


public class RideMgrTest
{
    private const string Email = "ana@example.com";
    private readonly RideMgr rides = new();


    private string Ride(string plan, int minutes, bool night)
    {
        return Money(rides.Calc(Email, plan, minutes, night));
    }


    private static string Money(double amount)
    {
        return amount.ToString("F2");
    }


    [Fact]
    public void PayAsYouGoRide()
    {
        Assert.Equal("3.00", Ride("payg", 10, false));
    }


    [Fact]
    public void StudentRide()
    {
        Assert.Equal("2.40", Ride("student", 10, false));
    }


    [Fact]
    public void MonthlyWithinFreeMinutes()
    {
        Assert.Equal("0.00", Ride("monthly", 30, false));
    }


    [Fact]
    public void MonthlyOverFreeMinutes()
    {
        Assert.Equal("3.00", Ride("monthly", 60, false));
    }


    [Fact]
    public void NightSurcharge()
    {
        Assert.Equal("3.50", Ride("payg", 10, true));
    }


    [Fact]
    public void FourthRideUnlockIsFree()
    {
        Ride("payg", 10, false);
        Ride("payg", 10, false);
        Ride("payg", 10, false);
        Assert.Equal("2.00", Ride("payg", 10, false));
    }


    [Fact]
    public void StudentFourthRide()
    {
        // As it is today: the discount is the full €1, not €0.80.
        Ride("student", 10, false);
        Ride("student", 10, false);
        Ride("student", 10, false);
        Assert.Equal("1.40", Ride("student", 10, false));
    }


    [Fact]
    public void Receipt()
    {
        Ride("payg", 10, false);
        Assert.Equal(new[] { "ana@example.com: 10 min, €3.00" }, rides.Out);
    }


    [Fact]
    public void Revenue()
    {
        Ride("payg", 10, false);
        Ride("student", 10, false);
        Assert.Equal("5.40", Money(rides.S));
    }
}
Result
PASS  FourthRideUnlockIsFree
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentFourthRide
PASS  StudentRide
Tests: 9, passed: 9, failed: 0

The tests call the class only through the Ride helper. When the shape of the class changes in the next steps, the helper and the calls will change, and the expected values never.

One of the tests is odd. A student pays 80% of the price, so their unlock costs €0.80. On the fourth ride, though, the class takes off the full €1, and the student pays €1.40 instead of €1.60. Is it a bug or a gift? You do not decide, and you do not decide now. The test pins what happens today, with a comment, and the question goes to the product owner. While you clean up, the behavior does not change, even when it looks wrong.

More precisely, if you are curious

The amounts are compared as text with two decimals ("2.40"), the way the customer sees them. Comparing numbers would trip over the small binary errors of double:

Console.WriteLine((1.0 + 10 * 0.2) * 0.8);
Output
2.4000000000000004

And in step 3 the type of the amounts will change. The text with two decimals is the same before and after.

See it

Step 1: the names and the flag

t, m, f, p, c and s become words (names). The most important name is the method's: it does not calculate, it finishes the ride. The honest name is FinishRide, and now the side effect shows in the name. The class becomes RideBilling.

public class RideBilling
{
    private readonly Dictionary<string, int> ridesToday = new();
    public double Revenue { get; private set; }
    public List<string> Receipts { get; } = new();


    // Price: €1 to unlock and €0.15 per minute.
    // At night the price goes up by 50%.
    public double FinishRide(string email, string plan, int minutes, bool night = false)
    {
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        double price = 0;
        if (plan == "payg")
        {
            price = 1.0 + minutes * 0.2;
        }
        else if (plan == "student")
        {
            price = (1.0 + minutes * 0.2) * 0.8;
        }
        else if (plan == "monthly")
        {
            if (minutes > 45)
            {
                price = (minutes - 45) * 0.2;
            }
        }
        if (ridesToday[email] > 3 && plan != "monthly")
        {
            price = price - 1.0;
        }
        if (night)
        {
            price = price + 0.5;
        }
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} min, €{price:F2}");
        return price;
    }
}

The flag is left (functions). true in the call says nothing. Night is data about the ride, not a choice of a different job, so it does not become two functions. The call has to say what it means:

C# has default values and named arguments: day has no argument, and night is night: true.
Before
private string Ride(string plan, int minutes, bool night)
{
    return Money(rides.Calc(Email, plan, minutes, night));
}


[Fact]
public void NightSurcharge()
{
    Assert.Equal("3.50", Ride("payg", 10, true));
}
After
private string Ride(string plan, int minutes, bool night = false)
{
    return Money(billing.FinishRide(Email, plan, minutes, night));
}


[Fact]
public void NightSurcharge()
{
    Assert.Equal("3.50", Ride("payg", 10, night: true));
}
The tests
PASS  FourthRideUnlockIsFree
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentFourthRide
PASS  StudentRide
Tests: 9, passed: 9, failed: 0
In Visual Studio, rename is Ctrl+R, Ctrl+R (or F2), in Rider it is Shift+F6: it changes the calls in the tests too.

See it

Step 2: the numbers and the comment that lies

1.0, 0.2, 0.8, 45, 3 and 0.5 become constants with names (magic number → constant).

The comment above the method says €0.15 per minute and a 50% night surcharge. The code charges €0.20 and adds €0.50. Which is true? The tests pinned what the code does, and customers have paid by it for years. The comment lies (comments) and is deleted. The constants say the same thing and cannot go stale on their own.

public class RideBilling
{
    private const double UnlockFee = 1.00;
    private const double PerMinute = 0.20;
    private const double StudentShare = 0.80;
    private const int MonthlyFreeMinutes = 45;
    private const int PaidUnlocksPerDay = 3;
    private const double NightSurcharge = 0.50;


    private readonly Dictionary<string, int> ridesToday = new();
    public double Revenue { get; private set; }
    public List<string> Receipts { get; } = new();


    public double FinishRide(string email, string plan, int minutes, bool night = false)
    {
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        double price = 0;
        if (plan == "payg")
        {
            price = UnlockFee + minutes * PerMinute;
        }
        else if (plan == "student")
        {
            price = (UnlockFee + minutes * PerMinute) * StudentShare;
        }
        else if (plan == "monthly")
        {
            if (minutes > MonthlyFreeMinutes)
            {
                price = (minutes - MonthlyFreeMinutes) * PerMinute;
            }
        }
        if (ridesToday[email] > PaidUnlocksPerDay && plan != "monthly")
        {
            price = price - UnlockFee;
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} min, €{price:F2}");
        return price;
    }
}
The tests
PASS  FourthRideUnlockIsFree
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentFourthRide
PASS  StudentRide
Tests: 9, passed: 9, failed: 0

The tests are not touched: neither the name nor the calls have changed.

Try it

Find the bug

A colleague does the same step, but trusts the comment and keeps it:

Pick the line with the bug.

More precisely, if you are curious
The tests
FAIL  FourthRideUnlockIsFree
FAIL  MonthlyOverFreeMinutes
FAIL  NightSurcharge
FAIL  PayAsYouGoRide
FAIL  Receipt
FAIL  Revenue
FAIL  StudentFourthRide
FAIL  StudentRide
PASS  MonthlyWithinFreeMinutes
Tests: 9, passed: 1, failed: 8

See it

Step 3: the money

Money in a double is primitive obsession (smells). Today it gives no wrong cent in any test, because every amount is rounded only when it is printed. That is why the step is a refactoring: the output is the same. But the revenue is a sum of thousands of such amounts, and that is where the small errors pile up.

double becomes decimal, and the literals get an m. The formulas stay as they are, because decimal has the same operators. In the course, money in C# was decimal from the start, but this class is old and was written with double.
public class RideBilling
{
    private const decimal UnlockFee = 1.00m;
    private const decimal PerMinute = 0.20m;
    private const decimal StudentShare = 0.80m;
    private const int MonthlyFreeMinutes = 45;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;


    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    public List<string> Receipts { get; } = new();


    public decimal FinishRide(string email, string plan, int minutes, bool night = false)
    {
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = 0;
        if (plan == "payg")
        {
            price = UnlockFee + minutes * PerMinute;
        }
        else if (plan == "student")
        {
            price = (UnlockFee + minutes * PerMinute) * StudentShare;
        }
        else if (plan == "monthly")
        {
            if (minutes > MonthlyFreeMinutes)
            {
                price = (minutes - MonthlyFreeMinutes) * PerMinute;
            }
        }
        if (ridesToday[email] > PaidUnlocksPerDay && plan != "monthly")
        {
            price = price - UnlockFee;
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} min, €{price:F2}");
        return price;
    }
}
In the tests only the helper that turns the amount into text changes:
private static string Money(decimal amount)
{
    return amount.ToString("F2");
}
The tests
PASS  FourthRideUnlockIsFree
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentFourthRide
PASS  StudentRide
Tests: 9, passed: 9, failed: 0

Try it

Fill in

The price per minute in the language's type for money:

private const decimal PerMinute = ;

See it

Step 4: the question apart from the command

Now the change that was waiting: the price before the end has to be a question that changes nothing (command or question). The calculation is extracted into a Price method (Extract Method from the refactoring lesson). FinishRide stays a command: it asks for the price, then counts, adds up and sends.

A colleague does it like this. The counter moves after the price, because a ride counts once it has finished:

What do the nine tests say?

public decimal Price(string email, string plan, int minutes, bool night = false)
{
    decimal price = 0;
    if (plan == "payg")
    {
        price = UnlockFee + minutes * PerMinute;
    }
    else if (plan == "student")
    {
        price = (UnlockFee + minutes * PerMinute) * StudentShare;
    }
    else if (plan == "monthly")
    {
        if (minutes > MonthlyFreeMinutes)
        {
            price = (minutes - MonthlyFreeMinutes) * PerMinute;
        }
    }
    if (ridesToday.GetValueOrDefault(email) > PaidUnlocksPerDay && plan != "monthly")
    {
        price = price - UnlockFee;
    }
    if (night)
    {
        price = price + NightSurcharge;
    }
    return price;
}


public decimal FinishRide(string email, string plan, int minutes, bool night = false)
{
    decimal price = Price(email, plan, minutes, night);
    ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
    Revenue = Revenue + price;
    Receipts.Add($"{email}: {minutes} min, €{price:F2}");
    return price;
}
Answer
More precisely, if you are curious
The tests
FAIL  FourthRideUnlockIsFree
      Assert.Equal() Failure: Strings differ
                 ↓ (pos 0)
      Expected: "2.00"
      Actual:   "3.00"
                 ↑ (pos 0)
FAIL  StudentFourthRide
      Assert.Equal() Failure: Strings differ
                 ↓ (pos 0)
      Expected: "1.40"
      Actual:   "2.40"
                 ↑ (pos 0)
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentRide
Tests: 9, passed: 7, failed: 2

See it

Step 4, second try

The red points at the last step. You do not look for the bug in the whole class: you undo the step and do it again. The order of the side effects was a hidden part of the behavior, and only the tests knew about it.

Price works out which ride of the day this will be:

public decimal Price(string email, string plan, int minutes, bool night = false)
{
    int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
    decimal price = 0;
    if (plan == "payg")
    {
        price = UnlockFee + minutes * PerMinute;
    }
    else if (plan == "student")
    {
        price = (UnlockFee + minutes * PerMinute) * StudentShare;
    }
    else if (plan == "monthly")
    {
        if (minutes > MonthlyFreeMinutes)
        {
            price = (minutes - MonthlyFreeMinutes) * PerMinute;
        }
    }
    if (rideOfDay > PaidUnlocksPerDay && plan != "monthly")
    {
        price = price - UnlockFee;
    }
    if (night)
    {
        price = price + NightSurcharge;
    }
    return price;
}


public decimal FinishRide(string email, string plan, int minutes, bool night = false)
{
    decimal price = Price(email, plan, minutes, night);
    ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
    Revenue = Revenue + price;
    Receipts.Add($"{email}: {minutes} min, €{price:F2}");
    return price;
}
The tests
PASS  FourthRideUnlockIsFree
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentFourthRide
PASS  StudentRide
Tests: 9, passed: 9, failed: 0

The tests are not touched again: they call FinishRide, and it does the same as before.

The whole class after step 4
public class RideBilling
{
    private const decimal UnlockFee = 1.00m;
    private const decimal PerMinute = 0.20m;
    private const decimal StudentShare = 0.80m;
    private const int MonthlyFreeMinutes = 45;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;


    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    public List<string> Receipts { get; } = new();


    public decimal Price(string email, string plan, int minutes, bool night = false)
    {
        int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = 0;
        if (plan == "payg")
        {
            price = UnlockFee + minutes * PerMinute;
        }
        else if (plan == "student")
        {
            price = (UnlockFee + minutes * PerMinute) * StudentShare;
        }
        else if (plan == "monthly")
        {
            if (minutes > MonthlyFreeMinutes)
            {
                price = (minutes - MonthlyFreeMinutes) * PerMinute;
            }
        }
        if (rideOfDay > PaidUnlocksPerDay && plan != "monthly")
        {
            price = price - UnlockFee;
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        return price;
    }


    public decimal FinishRide(string email, string plan, int minutes, bool night = false)
    {
        decimal price = Price(email, plan, minutes, night);
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} min, €{price:F2}");
        return price;
    }
}

See it

Step 5: the plans

What is left is the chain of if on the text of the plan (long if chain). Every new plan is a new branch, and "monthly" is checked in two places. Each plan becomes a class that knows its price and its discount for the fourth ride. This is Replace Conditional with Polymorphism from the refactoring lesson:

public interface IPlan
{
    decimal Price(int minutes);


    decimal LoyaltyDiscount();
}


public class PayAsYouGo : IPlan
{
    public decimal Price(int minutes)
    {
        return RideBilling.UnlockFee + minutes * RideBilling.PerMinute;
    }


    public decimal LoyaltyDiscount()
    {
        return RideBilling.UnlockFee;
    }
}


public class Student : IPlan
{
    private const decimal StudentShare = 0.80m;


    public decimal Price(int minutes)
    {
        return new PayAsYouGo().Price(minutes) * StudentShare;
    }


    public decimal LoyaltyDiscount()
    {
        // As it is today: the full €1, even though a student pays €0.80 to unlock.
        return RideBilling.UnlockFee;
    }
}


public class Monthly : IPlan
{
    private const int FreeMinutes = 45;


    public decimal Price(int minutes)
    {
        return Math.Max(0, minutes - FreeMinutes) * RideBilling.PerMinute;
    }


    public decimal LoyaltyDiscount()
    {
        return 0;
    }
}

The check for the monthly pass disappears: it simply returns a discount of 0. The student's odd behavior now sits on one line, with a comment that says why it is so (the good comment from the lesson on comments).

public class RideBilling
{
    public const decimal UnlockFee = 1.00m;
    public const decimal PerMinute = 0.20m;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;


    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    public List<string> Receipts { get; } = new();


    public decimal Price(string email, IPlan plan, int minutes, bool night = false)
    {
        int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = plan.Price(minutes);
        if (rideOfDay > PaidUnlocksPerDay)
        {
            price = price - plan.LoyaltyDiscount();
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        return price;
    }


    public decimal FinishRide(string email, IPlan plan, int minutes, bool night = false)
    {
        decimal price = Price(email, plan, minutes, night);
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        Revenue = Revenue + price;
        Receipts.Add($"{email}: {minutes} min, €{price:F2}");
        return price;
    }
}
The student share and the free minutes move into the plan that uses them. The unlock fee and the price per minute are shared by several plans, so they stay in the billing class and are no longer private.

In the tests the calls pass an object instead of text:

Assert.Equal("3.50", Ride("payg", 10, night: true));
Assert.Equal("3.50", Ride(new PayAsYouGo(), 10, night: true));
The tests
PASS  FourthRideUnlockIsFree
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentFourthRide
PASS  StudentRide
Tests: 9, passed: 9, failed: 0

This is Strategy from the patterns module: the rule is a separate object, and billing only asks.

See it

The two changes now

The business plan is a new class. The billing class is not touched (open for extension):

public class Business : IPlan
{
    private const decimal PerMinute = 0.18m;


    public decimal Price(int minutes)
    {
        return minutes * PerMinute;
    }


    public decimal LoyaltyDiscount()
    {
        return 0;
    }
}

The price before the end is the Price question. However many times you call it, nothing is counted and no receipt goes out:

var billing = new RideBilling();
var payg = new PayAsYouGo();
billing.FinishRide("ana@example.com", payg, 10);
billing.FinishRide("ana@example.com", payg, 5);
billing.FinishRide("ana@example.com", payg, 12, night: true);
Console.WriteLine($"Price before the end: €{billing.Price("ana@example.com", payg, 8):F2}");
Console.WriteLine($"Price before the end: €{billing.Price("ana@example.com", payg, 8):F2}");
Console.WriteLine($"Charged: €{billing.FinishRide("ana@example.com", payg, 8):F2}");
Console.WriteLine($"Company, 12 min: €{billing.FinishRide("office@example.com", new Business(), 12):F2}");
Console.WriteLine($"Receipts: {billing.Receipts.Count}");
Output
Price before the end: €1.60
Price before the end: €1.60
Charged: €1.60
Company, 12 min: €2.16
Receipts: 5

Five receipts: three rides before the preview, the fourth one and the company's. The two previews leave no trace, and they show exactly what is charged afterwards.

The idea

Before and after

Smell Step Lesson
nothing guards the behavior 0: tests that pin today's behavior the safety net
one-letter names, calc that does not say it sends 1: Rename names
true in the call 1: an argument that says what it means functions
magic numbers 2: constants with names names
a comment that lies 2: deleted comments
money in a double 3: a type for money smells
a question with a hidden side effect, a long method 4: Extract Method, command and question functions, refactoring
a chain of if on the plan as text 5: Replace Conditional with Polymorphism smells, refactoring, SOLID

The class got longer, not shorter. But every rule is in one place and has a name, and each change from the start is one class.

The day of rides, run with the class from before step 1 and with the class after step 5, prints the same thing line by line. Here both programs run on every check of the site.

Before
var rides = new RideMgr();
rides.Calc("ana@example.com", "payg", 10, false);
rides.Calc("ivan@example.com", "student", 25, false);
rides.Calc("ana@example.com", "payg", 5, false);
rides.Calc("maria@example.com", "monthly", 60, true);
rides.Calc("ana@example.com", "payg", 12, true);
rides.Calc("ana@example.com", "payg", 8, false);


foreach (string receipt in rides.Out)
{
    Console.WriteLine(receipt);
}
Console.WriteLine($"Revenue: €{rides.S:F2}");
After
var billing = new RideBilling();
var payg = new PayAsYouGo();
billing.FinishRide("ana@example.com", payg, 10);
billing.FinishRide("ivan@example.com", new Student(), 25);
billing.FinishRide("ana@example.com", payg, 5);
billing.FinishRide("maria@example.com", new Monthly(), 60, night: true);
billing.FinishRide("ana@example.com", payg, 12, night: true);
billing.FinishRide("ana@example.com", payg, 8);


foreach (string receipt in billing.Receipts)
{
    Console.WriteLine(receipt);
}
Console.WriteLine($"Revenue: €{billing.Revenue:F2}");
Output
ana@example.com: 10 min, €3.00
ivan@example.com: 25 min, €4.80
ana@example.com: 5 min, €2.00
maria@example.com: 60 min, €3.50
ana@example.com: 12 min, €3.90
ana@example.com: 8 min, €1.60
Revenue: €18.80

The idea

Where to stop

You stop when the changes that are coming become easy. Here the two changes are one class and one line. That is why some steps are deliberately not done:

  • the receipt in its own class. The text of the receipt is a second reason for the class to change (one reason to change). Today it does not change. As soon as marketing starts editing it every month, it becomes a separate class (task 1);
  • sending behind an interface. The receipts are a list, not a real email. When the real email arrives, the test must not send it, and that is when the abstraction comes (depending on an abstraction);
  • the student's behavior. That is a change in behavior, not a cleanup. It is done separately, with its own test, when the product decides.

And the rule from the refactoring lesson: either you change the shape or the behavior, never both in one step.

Exercise

Carry on yourself

Solve the tasks in your editor, starting from the class after step 5. After each step, run the nine tests.

1. The receipt in its own class. Move the text of the receipt and the list of sent receipts into a ReceiptBook class. The billing class only tells it "send". The tests are not touched.

One possible solution
public class ReceiptBook
{
    public List<string> Sent { get; } = new();


    public void Send(string email, int minutes, decimal price)
    {
        Sent.Add($"{email}: {minutes} min, €{price:F2}");
    }
}
public class RideBilling
{
    public const decimal UnlockFee = 1.00m;
    public const decimal PerMinute = 0.20m;
    private const int PaidUnlocksPerDay = 3;
    private const decimal NightSurcharge = 0.50m;


    private readonly Dictionary<string, int> ridesToday = new();
    public decimal Revenue { get; private set; }
    private readonly ReceiptBook receipts = new();
    public List<string> Receipts => receipts.Sent;


    public decimal Price(string email, IPlan plan, int minutes, bool night = false)
    {
        int rideOfDay = ridesToday.GetValueOrDefault(email) + 1;
        decimal price = plan.Price(minutes);
        if (rideOfDay > PaidUnlocksPerDay)
        {
            price = price - plan.LoyaltyDiscount();
        }
        if (night)
        {
            price = price + NightSurcharge;
        }
        return price;
    }


    public decimal FinishRide(string email, IPlan plan, int minutes, bool night = false)
    {
        decimal price = Price(email, plan, minutes, night);
        ridesToday[email] = ridesToday.GetValueOrDefault(email) + 1;
        Revenue = Revenue + price;
        receipts.Send(email, minutes, price);
        return price;
    }
}
The tests
PASS  FourthRideUnlockIsFree
PASS  MonthlyOverFreeMinutes
PASS  MonthlyWithinFreeMinutes
PASS  NightSurcharge
PASS  PayAsYouGoRide
PASS  Receipt
PASS  Revenue
PASS  StudentFourthRide
PASS  StudentRide
Tests: 9, passed: 9, failed: 0

The program for the day from the "Before and after" card prints the same output with this class.

2. The plan from text. The plan comes from the database as text ("payg"). Write the table that turns it into an object. What did the old class do with an unknown plan? Pin it with a test on the old class before you decide what the new one should do.

3. Night from the hour. The app no longer knows when it is night; it passes the start time. The rule "from 22:00 to 6:00" has to live in one place. Write the tests for 21:59, 22:00, 5:59 and 6:00 first.

4. The student. The product has decided: on the fourth ride a student gets €0.80, not €1. That is a change in behavior: first change the expected value in the test and see it red, then change one line in the plan.

Test

Check yourself

1. The test for a student on the fourth ride expects €1.40, and the "right" answer looks like €1.60. What do you do while you clean up the class?
2. The comment says €0.15 per minute, and the code charges €0.20. Which one do you trust?
3. Why do the tests call the class through one helper and not directly?
4. The first try at step 4 turned two tests red. What do you do?
5. After the cleanup, the business plan is one new class, and the billing class is not touched. Which principle is this?

Made by Ivo Gergov as part of Evo Tech's campaign for accessible programming education ·