Roy Osherove’s SOLID Kata String Calculator Redux

Roy Osherove recently posted a challenge to refactor a simple StringCalculator using SOLID principles. Sounds fun to me!

Here’s his original gist showing a StringCalculator.

{% gist 5098336 %}

Responsibilities

1. It is parsing string input into a set of integer values
2. It is performing the sum operation
3. It is defining a default result in the absence of valid input

Creating IntParser

public class IntParser
{
    public IEnumerable Parse(string input)
    {
        int x;

        foreach(var part in input.Split(','))
        {
            if (int.TryParse(part, out x))
                yield return x;
        }
    }
}

Integrating IntParser

public class StringCalculator
{
    public int DEFAULT_RESULT = 0;

    private readonly IntParser parser;

    public StringCalculator(IntParser parser)
    {
        this.parser = parser; // you could guard against null if you want
    }

    public int Add(string input)
    {
        var ints = this.parser.Parse(input);

        // Uh-oh. The way I've implemented this, I still pass the tests
        // but I have eliminated the explicit check for empty=>0.
        // It is only 0 by an implementation coincidence.This is however
        // irrelevant to the SOLID part of this exercise, so I'll leave it
        return ints.Sum();
    }
}

What have we achieved here? Well, the StringCalculator class is now immune to changes in how we handle or parse input. As long as IntParser will deliver some set of integers, it can add them. Is it SOLID now? While it’s better, we still have a long way to go. Let’s assess:

• SRP - check, we have definitely separated the parsing
• OCD - fail, this is neither open to extension nor closed to modification; if we wanted to handle new input formats, we cannot easily replace IntParser without changing StringCalculator
• LSP - defer, as we see no subtypes we cannot analyze the effects of substitution
• ISP - defer, as we have no interfaces, we cannot analyze their division
• DIP - fail, we are depending on the "concretion" of IntParser

Extracting an interface from IntParser, we get the IParseInts interface. StringCalculator can now depend on this interface instead of the specific IntParser implementation.

At this time, let’s also rename IntParser to a more specific IntsFromCommaSeparatedString. This name is somewhat lengthy, but naming is hard and it’ll do for now.

public interface IParseInts
{
    IEnumerable Parse(string input);
}

public class StringCalculator
{
    public int DEFAULT_RESULT = 0;

    private readonly IParseInts parser;

    public StringCalculator(IParseInts parser)
    {
        this.parser = parser;
    }

    public int Add(string input)
    {
        var ints = this.parser.Parse(input);
        return ints.Sum();
    }
}

What does this gain us? Well, now when we write unit tests around StringCalculator we can mock out IParseInts and have a reliable set of ints, even if our parser implementation regresses.

We are also able to extend our system to handle new formats without changing the existing calculator or parser. We’d simply pass the appropriate parser into the constructor. We could add a CanHandle(input) method to our interface and pass all registered parsers into the calculator, and let it ask on a case by case basis which one is appropriate. There are many ways to go here so I will leave it as an exercise for the reader or for a future blog post.

So our responsibilities are separated (SRP), we can extend the system to new formats without changing the existing implementations (OCD), our interface is small and specific to a task (ISP), and the calculator depends on an abstraction rather than a specific concrete class (DIP). We still have no subtypes, but whatever.

IDefault

So I made an IDefault interface and let StringCalculator depend on this as well. I made two implementations, the SimpleDefaulter which returns default(T) directly, and the ExceptionDefaulter which throws. (As an exercise, you could add a constructor to ExceptionDefaulter which accepts the message to use on the Exception, or a Func<Exception> that will construct the Exception on demand, or a Func<string, Exception> that would do the same but be able to utilize the string input…the sky is your limit).

public interface IDefault
{
    T GetDefault(string input);
}

public class SimpleDefault : IDefault
{
    public T GetDefault(string input)
    {
        return default(T);
    }
}

public class ExceptionDefault<TValue, TException> : IDefault
    where TException : Exception,new()
{
    public TValue GetDefault(string input)
    {
        throw new TException();
    }
}

public class StringCalculator
{
    private readonly IDefault<int> defaulter;
    private readonly IParseInts parser;

    public StringCalculator(IParseInts parser, IDefault<int> defaulter)
    {
        this.parser = parser;
        this.defaulter = defaulter;
    }

    public int Add(string input)
    {
        var ints = this.parser.Parse(input);
        if (!ints.Any())
            return this.defaulter.GetDefault(input);

        return ints.Sum(x => x);
    }
}

One might have been tempted to add this behavior to the IParseInts interface - after all, the default is only used in the case when the parsed ints are empty. However, we have practiced ISP by creating a separate, specific interface. Some systems might need defaults without parsing, or vice versa. Now either interface can be implemented without the baggage of the others!

Going Further?

• This is currently an implementation detail of our IntParser class. It eats parts that cannot be parsed as ints.
• We could change the spec of that implementation, but probably the "business" will want this to be consistent regardless of the parser or input format so...
• We could have an IValidateInput interface, or add an IsValid to the existing IParseInts interface. IValidateInput is probably a more strict following of ISP, but could be overkill. In either case, I think the concrete implementation would have the responsibility of implementing both methods - the choice in my mind is simply whether it has to implement two interfaces or just one.

Conclusion

{% gist 5111493 %}

I hope you enjoyed reading about this experience as much as I enjoyed going through the exercise. This is nothing groundbreaking but perhaps it will help SOLID click for someone. Also, I’d be very interested in any comments about what I might have missed or mistaken, or links to your #solidkata exercises so that I can learn how others approach the task. Happy coding!

This post originally appeared on The DevStop.