Search code examples
c#oopsingle-responsibility-principle

Understand Single Responsibility Principle of SOLID design


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.


Solution

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