Jerseys (CCC 2015)

I thought I was done after the first hour until I ran CCC’s test cases. Turns out I lacked a SINGLE but necessary test case that I didn’t write.

This is much more interesting case study of TDD and how skipping a step ahead can mess you up tremendously.

Problem
Given a list of jerseys with numbered from 1-J, and a list of requests from athlete for a specific jersey number and size, return the maximum number of jerseys that can be given out.

public class TestJerseys
{
    [Test]
    public void JerseyMaximizerTests()
    {
        List<JerseySize> jerseyInventory = new List<JerseySize>();
        List<JerseyRequest> jerseyRequests = new List<JerseyRequest>();
        JerseyRequestMaximizer maximizer =
            new JerseyRequestMaximizer(jerseyInventory, jerseyRequests);
        var maxJersies = maximizer.Maximize();
        Assert.That(maxJersies, Is.EqualTo(0));
    }
}

I figured I needed two vectors: one to told the inventory, one for the requests. I would also have a maximizer class to do the counting. The skeleton implementation was as follows:

public class JerseyRequestMaximizer
{
    private List<JerseyRequest> jerseyRequests;
    private List<JerseySize> jerseyInvertory;

    public JerseyRequestMaximizer(List<JerseySize> jerseyInventory, List<JerseyRequest> jerseyRequests)
    {
        this.jerseyInvertory = jerseyInventory;
        this.jerseyRequests = jerseyRequests;
    }

    public int Maximize()
    {
        return 0;
    }
}

public enum JerseySize
{
    Small,
    Medium,
    Large
}
public class JerseyRequest
{
    public JerseySize Size;
    public int Index;
}

Good it passes. After some refactoring to structure the test fixture I ended up with another test: if there is one small jersey in inventory (indexed 1), then requesting #1 small should maximize to 1:

[Test]
public void OneSmallShirtInInventoryWithSameReqestShouldReturn1()
{
    jerseyInventory.Add(JerseySize.Small);
    jerseyRequests.Add(new JerseyRequest(JerseySize.Small, 1));
    VerifyMaxJeries(maximizer.Maximize(), 1);
}
Now I jumped to the algorithm immediately here, as it was fairly obvious and easy enough to not make a mistake. I knew it wasn’t the correct solution but it was sufficient to make the test case pass while being a stepping stone to the next test case:
public int Maximize()
{
    int total = 0;
    foreach (var request in jerseyRequests)
    {
        var size = jerseyInvertory[request.Index - 1];
        if (size == request.Size)
            ++total;
    }
    return total;
}

The next test was to make sure that if I have two of the same request I need to keep track what jersey(s) was given out.

[Test]
public void OneSmallShirtInInventoryWithTwoSameRequestsShouldReturn1()
{
    AddJersey(JerseySize.Small);
    AddJerseyRequest(JerseySize.Small, 1);
    AddJerseyRequest(JerseySize.Small, 1);
    VerifyMaxJersies(maximizer.Maximize(), 1);
}
So far so good, minimal steps and making minimal tests pass.
public int Maximize()
{
    maxTotal = 0;
    foreach (var request in jerseyRequests)
    {
        if (request.Index > jerseyInventory.Count) continue;
        TryRequestJersey(request);
    }
    return maxTotal;
}

private void TryRequestJersey(JerseyRequest request)
{
    var index = request.Index - 1;
    var size = jerseyInventory[index];
    if (size == request.Size)
    {
        jerseyInventory.RemoveAt(index);
        ++maxTotal;
    }
}

Now there are huge problems with this code. Let’s see the next test first:

[Test]
public void TwoSmallShirtsInInventoryWithRepeatRequestShouldReturn1()
{
    AddJersey(JerseySize.Small);
    AddJersey(JerseySize.Small);
    AddJerseyRequest(JerseySize.Small, 1);
    AddJerseyRequest(JerseySize.Small, 1);
    VerifyMaxJersies(maximizer.Maximize(), 1);
}

This failed. It is because I’m using the List<> natural index as the jersey number. If I remove an element from the List the index gets uprooted. At this point I thought to myself perhaps I can introduce a 4th enum, something like “Removed” (along with Small, Medium , and Large) to keep track of what was given away. But that’s not good program design. So I used a dictionary instead. Here’s the complete JerseyRequestMaximizer at the time:

public class JerseyRequestMaximizer
{
    private List<JerseyRequest> jerseyRequests;
    private Dictionary<int, JerseySize> jerseyInventory;
    private int maxTotal;

    public JerseyRequestMaximizer(Dictionary<int, JerseySize> jerseyInventory, List<JerseyRequest> jerseyRequests)
    {
        this.jerseyInventory = jerseyInventory;
        this.jerseyRequests = jerseyRequests;
    }

    public int Maximize()
    {
        maxTotal = 0;
        foreach (var request in jerseyRequests)
        {
            if (request.Index > jerseyInventory.Count) continue;
            TryRequestJersey(request);
        }
        return maxTotal;
    }

    private void TryRequestJersey(JerseyRequest request)
    {
        if (!jerseyInventory.ContainsKey(request.Index)) return;
        var size = jerseyInventory[request.Index];
        if (size == request.Size)
        {
            jerseyInventory.Remove(request.Index);
            ++maxTotal;
        }
    }
}

At this point I thought my tests were sufficient to cover all cases except shirt sizes. But boy was I wrong… Anyways I preceeded with what I though would be my last test case:

[Test]
public void OneLargeShirtInInventoryWithOneSmallerRequestShouldReturn1()
{
    AddJersey(JerseySize.Large);
    AddJerseyRequest(JerseySize.Medium, 1);
    VerifyMaxJersies(maximizer.Maximize(), 1);
}

And making this pass was simply changing an if statement:

if (request.Size <= size)

And bam! All my test case pass. So I took some time to write out the file io code (with no tests unfortunately) to run the CCC test case. Annnnnd that through me off to a wile goose chase. Here’s what happened:

  1. Test 1-3 all passed
  2. Test 4 returned 376 where as expected 462
  3. First I thought I understood the question wrong, maybe they meant a unique jersey is its number AND size, so for example a request for #1 large and #1 medium would maximize to 2 if #1 was available in large
  4. I modified the program accordingly and it ended up with over 500…
  5. Feeling frustrated, I went to the solutions… it had the same idea as I did except it used the hacky way of using a T as a size placeholder method. I had the exact same algorithm! What the heck did I do wrong?
  6. I reverted my changes from 3 and started writing tests. I figured if I keep writing incremental tests it will catch something. But no… they were all freaking fine…
  7. Many tests later I ended up with this FAILING TEST:
[Test]
public void TwoSmallShirtsInInventoryWithSameRequestShouldReturn2()
{
    AddJersey(JerseySize.Small);
    AddJersey(JerseySize.Small);
    AddJerseyRequest(JerseySize.Small, 1);
    AddJerseyRequest(JerseySize.Small, 2);
    VerifyMaxJersies(maximizer.Maximize(), 2);
}

Simply put, if the inventory has #1 small and #2 small, and I request #1 small and #2 small, the maximizer should return 2. AND THIS FUCKING FAILED. Why did I skip this test? Oh it was because I jumped to the freaking algorithm with the for loop. My test only needed one item in the inventory and ONE REQUEST to pass. But my brain somehow told me “you got a damn for loop, you are covered for ALL cases!”. Then I just wrote some careless tests that I thought I tested multiple requests (because if 2 requests work it will work for n requests). And I just forgot about this degenerate test case. It was easy to spot the error after this:

if (request.Index > jerseyInventory.Count) continue;

This was here because of the List. But because I changed it to a Dictionary this no longer applies and screws up the program.

The Lesson
I think the problem came from inexperience with figuring out what are the most degenerative test cases and skipping ahead in TDD. Had I wrote that specific test first I would have caught the problem very early and me skipping to the algorithm would have been fine. In the future, I’m going to reduce the skipping scope to not jump to loops until I have a sufficient amount of if statements to do so.

Finally
Here is my source file for your viewing pleasure: TestJerseys.7z

Blog Comments powered by Disqus.

Previous Post