0

This question already has an answer here:

I'm still learning C# and best practices around it.

Consider this block of code

public class Counter {
    private int _value = 0;

    public void SetValue(int x) {
        _value = x;

        // Validation check
        if (_value < 0) _value = 0;
    }

    public Counter() {
        _value = SomeSingleton.Instance.GetDefaultCounterValue();
    }

    public void Increment() {
        SetValue(_value + 1);
    }

    public void Decrement() {
        SetValue(_value - 1);
    }

    public void LogValue() {
        Console.WriteLine(_value);
    }
}

note that for simple class like this, it is usually good to mix data and logic inside one class.

However this is not scalable and probably violating single responsibility principle (based on my poor understanding), as this class is responsible to:

  1. Hold the value data (poor-man database?).
  2. Doing some validation check on the value.
  3. Get the value from SomeSingleton.
  4. Operate on the value.
  5. Log the value.

What is the simplest way to separate data and logic for this code to adhere Single Responsibility Principle?

Unique explanation: I wanted to borrow functional programming paradigm to separate data and logic (in this case to different classes), also I'm unable to read other languages such as Java or python, I'm still learning C# myself. can we specify this on C# language only?

marked as duplicate by gnat, Greg Burghardt, BobDalgleish, Robert Harvey Aug 19 at 14:01

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

1

Data and logic do not need to be separated to satisfy SRP. In fact, I do not think you should be doing that, not even for functional programming. Yet, will get to that.

About your code, at method level you are not breaking SRP. However, at class level you are. Let us speak in reasons to change...

You could have to change your code if:

  • You decide to change your singleton solution※
  • You decide to change your logging solution (eg. EventLog, Log4net, etc...)
  • You decide to change how you manipulate the data (including validation)

※: I understand that the singleton is encapsulated. However the code still needs to know what singleton to use, and if you decide you want a different one, that is a reason to change.

These reasons to change are more than one, and two of them depend on external factors (the singleton and the log), that are likely to change independently. That is not good.

The usaul approaches to solve these are:

  • Pass the initial value on the constructor. If you really need that singleton (I doubt you do), have a factory that takes it from the singleton and pass it to the constructor.
  • Either allow to pass a log callback to the constructor or expose the value so that it can be logged externally※※.

※※: I would suggest to use the getter. Although you should avoid deciding on getters (in particular if we consider multiple threads accesing the object concurrently). For example client code should not have to read the getter to decide if it can decrement the value, instead the class should know. It is Tell, Don't Ask... getters can still be good for logging and debugging. I prefer the getter because the log is external to the system.


In practice, you want to encapsulate state and behavior together. Every proponent of object oriented design will argue in favor of encapsulation, because "The last thing you wanted any programmer to do is mess with internal state even if presented figuratively. Instead, the objects should be presented as sites of higher level behaviors more appropriate for use as dynamic components." -- Alan Kay.

Yes, Alan Kay won't like your public SetValue. He would also not like the idea of separating data..


I wanted to borrow functional programming paradigm to separate data and logic

To separate the data from the behavior would mean a naive approach to get functional programming: Have the class be static and all the methods that operate on the value be simple in-and-out methods. That is not an object oriented desing.

However, that does not mean that functional programming is against custom data types. In fact, it would encourage them. However, they would be immutable. Think of the object as a smart value that comes with the operations that you can do with it. Similarly to how you operate on an int, each operations yields a new value. Sure, you can have variables of its type and change their value. However, similar to how 0 will remain 0, the objects of your immutable type remain what they are.

Thus, you can embrace functional programming principles, and remain with an object oriented design, by using an immutable type.

4

From OOP and C# perspective the example code doesn't violate SRP. Data and basic operations over it belong together, that's what classes are. The idea to separate data and logic should be applied on higher level in large applications, e.g. business logic should be separate from data persistence.

One can make other remarks on the code, not related to SRP. Having public method SetValue breaks encapsulation. Also, the usage of singleton is suspicious, it looks like dependency injection should be used in that case.

  • 1
    I really like another viewpoint of the SRP from Robert Martin: "Gather together the things that change for the same reasons. Separate those things that change for different reasons." – Greg Burghardt Aug 16 at 11:55
  • @GregBurghardt Without further description of the word "reason", anyone can interpret that quote to mean literally anything. Show me any two lines of code and I can tell you reasons they may change together as well as reasons that would only change one. – Robert Bräutigam Aug 25 at 15:34

Not the answer you're looking for? Browse other questions tagged or ask your own question.