Theatrical players refactoring Kata

Kata based on the work from Emily Bache.

Objectives

Learn how to refactor “legacy” code
Practice OOP Design Patterns
Practice FP concepts

How to

Clone this repository : https://github.com/ythirion/Theatrical-Players-Refactoring-Kata

Exercise

Add an HTML output with the same information

Facilitation

What do you think about this code ?

StatementPrinter.java

import java.text.NumberFormat;
import java.util.Locale;
import java.util.Map;

public class StatementPrinter {

    public String print(Invoice invoice, Map<String, Play> plays) {
        var totalAmount = 0;
        var volumeCredits = 0;
        var result = String.format("Statement for %s\n", invoice.customer);

        NumberFormat frmt = NumberFormat.getCurrencyInstance(Locale.US);

        for (var perf : invoice.performances) {
            var play = plays.get(perf.playID);
            var thisAmount = 0;

            switch (play.type) {
                case "tragedy":
                    thisAmount = 40000;
                    if (perf.audience > 30) {
                        thisAmount += 1000 * (perf.audience - 30);
                    }
                    break;
                case "comedy":
                    thisAmount = 30000;
                    if (perf.audience > 20) {
                        thisAmount += 10000 + 500 * (perf.audience - 20);
                    }
                    thisAmount += 300 * perf.audience;
                    break;
                default:
                    throw new Error("unknown type: ${play.type}");
            }

            // add volume credits
            volumeCredits += Math.max(perf.audience - 30, 0);
            // add extra credit for every ten comedy attendees
            if ("comedy".equals(play.type)) volumeCredits += Math.floor(perf.audience / 5);

            // print line for this order
            result += String.format("  %s: %s (%s seats)\n", play.name, frmt.format(thisAmount / 100), perf.audience);
            totalAmount += thisAmount;
        }
        result += String.format("Amount owed is %s\n", frmt.format(totalAmount / 100));
        result += String.format("You earned %s credits\n", volumeCredits);
        return result;
    }

}

Will it be easy to do the exercise ?
Why ?

Identify code smells

Does it break any S.O.L.I.D principles ?

How to start ?

"Before you start refactoring, make sure you have a solid suite of tests. Theses tests must be self-checking." - Martin Fowler

Which kind of tests can we do ?

The code already exists and works :

Easiest way to add a regression test is to find some test data, exercise the code, and approve the result
Add approval tests / snapshot tests / Golden master

Approval tests : generate output / create your golden master

We store this golden master in a file :

StatementPrinterTests.exampleStatement.approved.txt

Statement for BigCo
  Hamlet: $650.00 (55 seats)
  As You Like It: $580.00 (35 seats)
  Othello: $500.00 (40 seats)
Amount owed is $1,730.00
You earned 47 credits

Approval tests : create a test

To do so we can use the library "approvaltests".

The verify is an approvaltests method that will compare the result returned by the print method and our Golden master.

StatementPrinterTests.java

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import java.util.List;
import java.util.Map;

import static org.approvaltests.Approvals.verify;

public class StatementPrinterTests {

    @Test
    void exampleStatement() {
        Map<String, Play> plays = Map.of(
                "hamlet",  new Play("Hamlet", "tragedy"),
                "as-like", new Play("As You Like It", "comedy"),
                "othello", new Play("Othello", "tragedy"));

        Invoice invoice = new Invoice("BigCo", List.of(
                new Performance("hamlet", 55),
                new Performance("as-like", 35),
                new Performance("othello", 40)));

        StatementPrinter statementPrinter = new StatementPrinter();
        var result = statementPrinter.print(invoice, plays);

        verify(result);
    }

Have we missed something ?

We should ask ourselves if we have covered every piece of code with our test.

To do so we have a tool : code coverage.

The Code Coverage (from IntelliJ here) shows us that the default case is not covered at the moment.

So let's add a new test :

StatementPrinterTests.java

        @Test
        void statementWithNewPlayTypes() {
            Map<String, Play> plays = Map.of(
                    "henry-v",  new Play("Henry V", "history"),
                    "as-like", new Play("As You Like It", "pastoral"));
        
            Invoice invoice = new Invoice("BigCo", List.of(
                    new Performance("henry-v", 53),
                    new Performance("as-like", 55)));
        
            StatementPrinter statementPrinter = new StatementPrinter();
            Assertions.assertThrows(Error.class, () -> {
                statementPrinter.print(invoice, plays);
            });
        }