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.

CodePudding user response：

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!

CodePudding user response：

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

CodePudding user response：

in a nutshell, single responsibility says a function must do just one thing. but in your example, you focused on passing arguments.

as far as I know, it doesn't matter whether your function is static or not.

And of course, it's better to work with interfaces in a real project. in this approach, you make your code reusable and comply with the dependency inversion principle too.

Regarding your question, I choose the second approach. I think it is better.