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.

Responsibilities

This thing is doing at least two things, and I contrived a third.

  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

I think perhaps it should only be responsible for the second of these. It is a calculator, so parsing should certainly be outside of its scope. We will save the definition of the default result for later. First, let’s separate parsing.

Creating IntParser

The first thing I did was create an IntParser with a single method. This method accepted a string of input and returned an IEnumerable of int values.

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

The simplest thing to do next was to accept an IntParser as a constructor parameter for the StringCalculator class. Now instead of the Add method checking the input and deciding how to get ints from it, it simply delegates that responsibility to the dependency.

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

The Abstraction of IParseInts

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

The other responsibility I contrived above was the determination of the default value. Something about having this as a public const was bothering me.  What if callers wanted to use a different default value, or perhaps even result in an exception if no sum could be calculated. How would they determine whether “0” was because the numbers added to zero, or because the input failed?

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?

We could go pretty crazy, but I think it’s good enough for now. A couple of things I might do differently if I wanted to spend more time on this? Here is one thing that immediately comes to mind. In the above, “a,b,c” and “David” will behave exactly the same as “”. The default will be executed whether the set is empty or invalid. Thoughts regarding this:

    • 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.

Update: Another thought is that while we have opened things up to support multiple string formats, we still only support string inputs. We could abstract this to instead of having IParseInts which parses a string, have something like IProvideInts, and instead of .Parse, have .RetrieveAll. The add method would then either accept an IProvideInts parameters instead of a string, or it would just directly accept the set of ints (having been obtained previously by the caller).

Conclusion

So here is my final gist.

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!

Advertisements

About David Ruttka

I've been "making computers do things" since I first saw King's Quest on a 286 PC in the mid-80's, but I turned it into a career just over a decade ago. While the majority of my experience has been on the Microsoft stack (C#, .NET, ASP.NET), I've recently been diving deeper into JavaScript and exploring the Ruby universe. Occasionally, I'll do a public speaking gig or write a blog post. When I'm not coding, I enjoy spending time with my family, watching hockey, and playing the occasional video game. You can also find me on Stack Overflow, Google Plus, and Twitter. Microsoft Certified Programming in HTML5 with JavaScript and CSS3 Specialist; MCPD Windows Developer 3.5
This entry was posted in Development and tagged , , . Bookmark the permalink.

2 Responses to Roy Osherove’s SOLID Kata–String Calculator Redux

  1. The IntParser is great.
    But everything else looks a bit like a demonstration how to use SOLID principles to refactor from simple to complex. So my peronal conclussion is : While there is value in solid principles I value simple more.

    • David Ruttka says:

      Hi Peter,

      Fair point =) In this trivial case and given the existing StringCalc requirements, I absolutely agree that IParseInt and IDefault are overkill. I am planning a follow-up post where I go more into when / if each principle should be applied – this post was an afternoon “coffee break” type of diversion and I realize I left a lot of that out.

      That said, taking the further steps has saved our proverbial bacon many times at my current employer. Each customer wants slightly different behaviors. The OCP and DIP have opened our system to easily extending or replacing certain parts without impacting or destabilizing others.

      There are trade offs at every corner I think. The point of this exercise (and most katas), I think, is to make the concepts more familiar on a small scale so that they can be recognized and applied when (if?) they are needed later. Knowing when (if?) is certainly key.

Thoughts?

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s