3

I'm a hobby coder trying to improve my code. I tend to create monolithic classes and want to start being more the S in SOLID. I've done some reading on here and elsewhere, but I'm struggling to get my head around what the best approach to doing this is. I can think of three scenarios:

  1. Static Methods
  2. Through instantiation
  3. Mixture of above but passing full parent class to dependency class (does this have memory implications or not due to it just being a pointer?)
namespace SingleResponsabilityTest
{
    internal class Program
    {
        static void Main(string[] args)
        {
            Factoriser factoriser = new Factoriser();
            factoriser.DoFactorTen();
        }
    }

    internal class Factoriser
    {
        public int classSpecificInt = 10;
        public void DoFactorTen()
        {
            SingleResponsabiltyApproach1 sra1 = new SingleResponsabiltyApproach1(classSpecificInt);
            Console.WriteLine(sra1.FactorTen());

            Console.WriteLine(SingleResponsabiltyApproach2.FactorTen(classSpecificInt));

            Console.WriteLine(SingleResponsabiltyApproach3.FactorTen(this));

            Console.ReadLine();
        }
    }

    internal class SingleResponsabiltyApproach1
    {
        int passedInt = 0;
        public SingleResponsabiltyApproach1(int passedInt)
        {
            this.passedInt = passedInt;
        }
        public int FactorTen()
        {
            return passedInt * 10;
        }
    }

    internal class SingleResponsabiltyApproach2
    {
        public static int FactorTen(int passedInt)
        {
            return passedInt * 10;
        }
    }

    internal class SingleResponsabiltyApproach3
    {
        public static int FactorTen(Factoriser factoriser)
        {
            return factoriser.classSpecificInt * 10;
        }
    }

}

What is the best approach?

Also, where does dependency injection and interfaces come into all this? Thanks.

stigzler
  • 793
  • 2
  • 12
  • 29
  • 1
    Just try to not get into extremes with this. The best approach is `Console.WriteLine(10 * 10)`. I know it's just an example but people often wrap simple things into enormous amounts of abstractions in real code, which is hard to read and understand (and navigate), and so is harder to maintain. – Evk Dec 17 '22 at 16:59
  • 1
    @Evk Your comment is an example of the tension I see repeatedly between a "make sure your code is easy to read" approach, versus a "make sure your code is easy to extend and modify without breaking other code" approach. Your comment also seems to miss the point of the question: which is about SRP (and how it interplays with the other SOLID design principles), and is not asking for the simplest code for factoring an integer. I personally have been struggling with learning SOLID and greatly appreciate this question, and especially the Answer provided by Olivier Jacot-Descombes. – wopr_xl Jan 29 '23 at 15:36
  • 1
    @wopr_xl that's just word of caution, because I've seen enough codebases spoiled by overuse of abstractions (because they are popular) and hard to maintain because of that. Once people learn those patterns (like OP does) they usually try to use them everywhere, so I'd suggest to use common sense and not try to find a way to use the abstraction where there might be no need to. – Evk Jan 30 '23 at 14:11
  • @Evk I hear you and completely agree. But, what is "common sense" to an experienced developer is different to what is "common sense" to someone learning. A newbie does not have the "benefit" of the painful experience of having to maintain someone else's code that is overrun with excessive abstractions. Once you experience that pain -- then yes, I'm sure it then becomes "common sense." Personally, I wish there were more BAD examples of using abstraction, to get a better sense of when you've crossed the line. Just goes to show that coding truly is as much "art" as it is "science." – wopr_xl Jan 31 '23 at 23:00

2 Answers2

3

You are abstracting over the value passedInt. This is not the right approach. You must split the functional responsibilities. Here I can detect 3 responsibilities:

  • Multiply (i.e., calculate)
  • Writing to the console (i.e., logging in the broadest sense)
  • Organizing and combining calculations and logging.

Therefore I declare 3 interfaces describing these 3 requirements:

public interface ICalculator
{
    int Multiply(int x, int y);
}

public interface ILogger
{
    void Log(string message);
    void Close();
}

public interface IFactoriser
{
    void DoFactorTen(int value);
}

Here is a possible implementation:

public class Calculator : ICalculator
{
    public int Multiply(int x, int y)
    {
        return x * y;
    }
}

public class ConsoleLogger : ILogger
{
    public void Log(string message)
    {
        Console.WriteLine(message);
    }

    public void Close()
    {
        Console.ReadKey();
    }
}

public class Factoriser : IFactoriser
{
    private ICalculator _calculator;
    private ILogger _logger;

    public Factoriser(ICalculator calculator, ILogger logger)
    {
        _calculator = calculator;
        _logger = logger;
    }

    public void DoFactorTen(int value)
    {
        int result = _calculator.Multiply(value, 10);
        _logger.Log($"The result is {result}");
        _logger.Close();
    }
}

Note that the Factoriser does not need to know the details about calculations and logging. Therefore these responsibilities are injected in the Factoriser through constructor injection. Note that we are injecting the responsibilities, not the values like classSpecificInt = 10 in your example. The implementations should be flexible enough to deal with all possible values.

Now, we can write the Main method like this:

static void Main(string[] args)
{
    var calculator = new Calculator();
    var logger = new ConsoleLogger();
    var factoriser = new Factoriser(calculator, logger);

    factoriser.DoFactorTen(15);
}

Now, you could easily write this result to a file by providing a file logger instead of a console logger. You could inject the file name into the logger through the constructor. In this case it makes sense to inject a value, because the logger will have to log into the same file during its whole lifetime.

This would not have an impact on the Factoriser, since an abstract ILogger is injected.

This approach implements these SOLID principles:

  • Single-responsibility principle (SRP).
  • Open–closed principle:
    • We can extend the behavior of our interfaces and implementations by deriving new interfaces and classes from them, i.e., without modifying the existing ones.
  • Liskov substitution principle (LSP):
    • We can inject a class derived from our calculator or logger or inject completely different implementations to the Factoriser without the Factoriser knowing it.
  • The Interface segregation principle (ISP):
    • Our interfaces declare a minimal API and thus our implementations do not depend on methods they do not use.
  • The Dependency inversion principle (DI):
    • Factoriser depends upon abstractions (i.e. interfaces), not concretions (i.e., not specific classes). Your implementation of Factoriser depends on concrete implementations because it calls, e.g.: new SingleResponsabiltyApproach1(..).

Note also that IFactoriser does not depend on the other interfaces. This gives us a high degree of flexibility in implementation.

Olivier Jacot-Descombes
  • 104,806
  • 13
  • 138
  • 188
  • 1
    This Answer is one of the clearest examples of implementing SOLID design principles that I've seen and is of enormous help to my understanding with respect to determining the "responsibilities" to segregate in this manner. I am curious whether you would have a response to the comment of Evk above regarding the trade-off as to easy "readability" versus easy "extend-ability"? This is an important and confusing aspect for people learning SOLID -- such as myself. – wopr_xl Jan 29 '23 at 15:48
  • 1
    All these principles are only guidelines. They require the implementer to use common sense in implementation. If you end up in having thousands of methods having one single line of code and thousands of interfaces and classes each one declaring one of these methods, then you've taken those principles ad absurdum. Finding the right balance is maybe the most difficult part in programming. – Olivier Jacot-Descombes Jan 29 '23 at 17:04
1

The Single Responsibility Principle (SRP) basically says:

A Class/method must have only one responsibility

So, to go over this principle, think about a class Car with a god method called TurnOn(). Inside this method, you start the car, turn the lights on, accelerate, and brake. And another method called TurnOff(), turning off engine, lights, and braking.

Car

  • TurnOn()
  • TurnOff()

If you need to use this class, you may think the method TurnOn() only turns the car on, which is not valid, and it breaks the SRP. The same applies for the TurnOff()

Applying the SRP, the class Car must have the methods:

Car

  • TurnEngineOn()
  • TurnEngineOff()
  • Accelerate()
  • Brake()
  • TurnLightsOn()
  • TurnLightsOff()

So now, if you need to use the Car class, you know exactly how to use each part of the car independently.

You can notice every method with a specific responsibility.

I changed a few things in your example to apply the SRP:

namespace SingleResponsibility
{
    internal class Program
    {
        // All the UI interaction (read, write) happens in the main method (UI layer with the user)
        // So the single responsibility of the UI is only calling methods and interacting with users
        static void Main(string[] args)
        {
            Console.Write("Tell me a number to factorise: ");
            int myNumber = Convert.ToInt32( Console.ReadLine() );

            SingleResponsabiltyApproach1 sra1 = new SingleResponsabiltyApproach1(myNumber);
            Console.WriteLine( $"My first approach {sra1.DoFactorTen()}" );

            SingleResponsabiltyApproach2 sra2 = new SingleResponsabiltyApproach2();
            Console.WriteLine($"My second approach {sra2.DoFactorTen(myNumber)}");

            Console.ReadLine();
        }
    }
       
    // The single responsibility of this class is to do an approach using a parametered constructor
    internal class SingleResponsabiltyApproach1
    {        
        // using property as private (encapsulated)
        private int PassedInt { get; set; }
        
        // starting the constructor with a parameter        
        public SingleResponsabiltyApproach1(int passedInt)
        {
            this.PassedInt = passedInt;
        }

        // doing the factor
        // The single responsibility of this method is to do a factor ten, and its name really means this
        public int DoFactorTen()
        {
            return PassedInt * 10;
        }
    }

    // The single responsibility of this class is to do an approach using a default constructor
    internal class SingleResponsabiltyApproach2
    {
        // no custom constructor

        // doing the factor passing number as parameter
        // The single responsibility of this method is to do a factor ten with a number
        // provided, and its name and signature really means this
        public int DoFactorTen(int passedInt)
        {
            return passedInt * 10;
        }
    }
}

If you want to go over interfaces and dependency injections, maybe you can go over the other more complex principles, such as Liskov Substitution Principle (LSK), Interface Segregation Principle (ISP), Dependency Inversion Principle (DIP).

Cheers!

  • Thank you. I think I was getting mixed up. I totally get SRP for a method should only have one responsibility. I was thinking that a Class should only have one method. My classes tend to be quite monolithic, so this is what I'm trying to work on - making smaller classes, but I can see from this example that you can have more than one method in a class. I totally get it for concrete models like Cars, but for more abstract Classes like Presenters and Services/Controllers, these tend to get very monolithic in my code... – stigzler Dec 17 '22 at 23:45