Understand Single Responsibility Principle of SOLID design

Question

I'm currently trying to learn the SOLID design principles along with behavior driven development, but am having quite the hard time getting my head around the Single Responsibility Principle. I've tried to find a good tutorial for c# that uses test driven development, but haven't been able to find anything worthwhile in that vein. Anyway, after spending a few days reading about it, I decided the best way to learn is by experience, so I started creating a small app using those principles best I can.

It's a simple bowling score calculator. I figured the best way to go about it was to work from the simplest part up, so I started at the ball (or throw) level. Right now I have created tests and an interface and class for dealing with ball(throw) scores, which makes sure they aren't invalid, ie. <10 or >0. After implementing this I realized that the ball class is basically just a nullable integer, so maybe I don't even need it... but for now it's there.

Following the ball, I decided the next logical thing to add was a frame interface and class. This is where I have gotten stuck. Here is where I'm at:

namespace BowlingCalc.Frames
{
    public interface IFrame : BowlingCalc.Generic.ICheckValid
    {
        void AddThrow(int? score);
        int? GetThrow(int throw_number);
    }
}

namespace BowlingCalc.Frames
{
    public class Frame : IFrame
    {
        private List<Balls.IBall> balls;
        private Balls.IBall ball;
        private int frame_number;

        public Frame(Balls.IBall ball, int frame_number)
        {
            this.ball = ball;
            this.frame_number = frame_number;
            balls = new List<Balls.IBall>();
            check_valid();
        }

        public void AddThrow(int? score)
        {
            var current_ball = ball;
            current_ball.Score = score;
            balls.Add(current_ball);
        }

        public int? GetThrow(int throw_number)
        {
            return balls[throw_number].Score;
        }

        public void check_valid()
        {
            if (frame_number < 0 || frame_number > 10)
            {
                throw (new Exception("InvalidFrameNumberException"));
            }
        }

    }
}

The frame uses my previously implemented ball class through dependency injection to add ball scores to the frame. It also implements a way to return the score for any given ball in the frame.

What I want to do, and where I'm stuck, is I want to add a way to make sure the frame score is valid. (For the moment just the simple case of frames 1-9, where the combined score of both balls must be 10 or less. I will move on to the much more complicated frame 10 case later.)

The problem is I have no idea how to implement this in a SOLID way. I was thinking of adding the logic into the class, but that seems to go against the single responsibility principle. Then I thought to add it as a separate interface/class and then call that class on each frame when its score updates. I also thought of creating a separate interface, IValidator or something like that, and implementing that in the Frame class. Unfortunately, I have no idea which, if any, of these is the best/SOLID way of doing things.

So, what would be a good, SOLID way to implement a method that validates the score of a frame?

---

Note: Any critique of my code is very welcome. I am very excited learn to create better code, and happy to receive any help given.

Answer 1

What is the purpose of ICheckValid interface? Do you call check_valid elsewhere? In my opinion, since the frame_number seems to be in fact a read-only property of a Frame, why it would be wrong to verify its consistency right in the constructor without any additional interfaces for that? (Constructors are supposed to produce consistent objects and are thus free to validate incoming parameters however they like.)

However, rather than to ask how to properly validate this field, it might be better to ask why indeed you need the frame_number property in the Frame? It seems like this is an index of this item in some array - you may just use the index, why store it in the Frame? You may want to write some if/else logic later, such as:

if (frame_number == 10) {
     // some rules
} else {
     // other rules
}

However, this is unlikely a SOLID approach as you would probably end up writing this if/else statements in many parts of the Frame. Rather, you may create a base class FrameBase, define much of the logics there plus some abstract methods to be implemented in OrdinaryFrame and TenthFrame, where you would define different rules. This would enable you to avoid frame_number altogether -- you would just create nine OrdinaryFrames and one TenthFrame.

As for critique: your code seems to abstract balls and frames, but ignores 'throws', or 'rolls' for some reason. Consider a need to add trajectory information of each roll, you would need to change the IFrame interface, adding something like void SetThrowTrajectory(int throwNumber, IThrowTrajectory trajectory). However, if you abstract throws away in an e.g. IBallRoll, the trajectory-related functionality would easily fit there (as well as some Boolean computed properties, e.g. IsStrike, IsSpare).

Answer 2

When I think SRP, I tend to put the emphasis on the Responsibility aspect. The name of the class, in turn, should ideally describe its Responsibility. For some classes this is about what the class is supposed to 'be' (a Frame could be a good example if it lacks behavior and merely represents state), but when you have a behavioral responsibility, the name is about what the class is supposed to 'do'.

Computing scores by itself is a fairly small responsibility, so let's consider something slightly larger and more naturally decomposable. Here is one possible breakdown of a bowling game with simple responsibilities and with suitably paranoid encapsulation (we're all friends here, but nobody wants anybody to cheat by mistake.)

//My job is to keep score; I don't interpret the scores
public interface IScoreKeeper : IScoreReporter
{
    void SetScore(int bowlerIndex, int frameIndex, int throwIndex, int score);
}

//My job is to report scores to those who want to know the score, but shouldn't be allowed to change it
public interface IScoreReporter
{
    int? GetScore(int bowlerIndex, int frameIndex, int throwIndex);        
}

//My job is to play the game when told that it's my turn
public interface IBowler
{
    //I'm given access to the ScoreReporter at some point, so I can use that to strategize
    //(more realisically, to either gloat or despair as applicable)

    //Throw one ball in the lane, however I choose to do so
    void Bowl(IBowlingLane lane);
}

//My job is to keep track of the pins and provide score feedback when they are knocked down
//I can be reset to allow bowling to continue
public interface IBowlingLane
{
    int? GetLastScore();

    void ResetLane();
}

//My job is to coordinate a game of bowling with multiple players
//I tell the Bowlers to Bowl, retrieve scores from the BowlingLane and keep
//the scores with the ScoreKeeper.
public interface IBowlingGameCoordinator
{
    //In reality, I would probably have other service dependencies, like a way to send feedback to a monitor
    //Basically anything that gets too complicated and can be encapsulated, I offload to some other service to deal with it
    //I'm lazy, so all I want to do is tell everybody else what to do.

    void PlayGame(IScoreKeeper gameScore, IEnumerable<IBowler> bowlers, IBowlingLane lane);
}

Note that if you wanted to use this model to simply compute scores (without playing a real game), you can have a stub Bowler (who does nothing) and a MockBowlingLane, who produces a series of score values. The BowlingGameCoordinator takes care of the current bowler, frame and throw, so the scores get accumulated.