1

It's a discussion about follow very strictly the SRP (Single Responsability Principle) vs. be more flexible when you write simple codes, like setting or getting properties values, even out of a direct context.

My first doubt is about where SRP should be included. I always read about SRP should be applied on classes. But and about methods, properties or traits (as in PHP), ...?

If this answer is no, maybe we can discard all text below.


To exemplify, let's define an example case. To be more easy understand, we'll write a singleton structure for a class called Example. Disregard IoC, for this case.

It should have two methods, basically:

  • getInstance(): returns an unique instance of your class; and
  • rebuildInstance(): it'll only destroy current instance and recreate it;

Basically to it works, getInstance() should check if the internal instance was set, create it if not, then return it. And rebuildInstance() should destroy current instance if it was defined and create a new, without return nothing.

Let's turn it more visual (in some pseudo-language):

class Example 
{
    static private Example instance;

    static public Example getInstance() 
    {
        if (!this.instance) 
        {
            this.instance = new Example;
        }

        return this.instance;
    }

    static public void rebuildInstance() 
    {
        if (this.instance)
        {
            destroy(this.instance);
        }

        this.instance = new Example;
    }
}

So note that:

  • getInstance() have two responsabilities: 1. check and instantiate; 2. return.
  • rebuildInstance() have two responsabilities: 1. check and destroy; 2. instantiate;

You will see that both should instantiate, so let consider it as an effect duplication. To avoid that, I should decouple my methods to each one have a single responsability, then I need creates two other methods:

  • buildInstanceWhenNeed();
  • destroyInstanceWhenNeed();

It'll turn more clear what each methods does by name and by its structure. So our code will seems like:

class Example 
{
    static private Example instance;

    static private void buildInstanceWhenNeed() 
    {
        if (!this.instance) 
        {
            this.instance = new Example;
        }
    }

    static public Example getInstance() 
    {
        self.buildInstanceWhenNeed();

        return this.instance;
    }

    static private void destroyInstanceWhenNeed()
    {
        if (this.instance)
        {
            destroy(this.instance);
        }
    }

    static public void rebuildInstance() 
    {
        self.destroyInstanceWhenNeed();
        self.buildInstanceWhenNeed();
    }
}

Okay.

Seems that now I'm respecting SRP completely on methods, very strictly. Each method have a single responsability without get out the class responsability. Except by the fact it have a singleton pattern inside of it, but I told to disregard IoC usage on start of topic.

So now I have additional questions:

  1. Should I apply SRP on methods, decoupling it, to make sure that it have a single responsability on each method?
  2. If yes. Should I do it even when the method don't duplicate any effect? For instance: if rebuildInstance() doesn't exists here, then I don't need decouple getInstance() because is the unique instance that will, in fact, create a new instance of class. So in this case, do I still need create buildInstanceWhenNeed() or not?
  3. If yes, but lower priority. And about performance? I mean, when it really affect the runtime, decreasing considerably the speed of application because of ultra decoupled methods (which perhaps only affects interpreted languages), but with high specifications to each one.
  • 1
    If all your code looks like your example, you don't really need classes at all, just go with procedural programming. Even better, SRP does not apply to procedural programming, problem solved. – Andy Jan 25 '16 at 18:26
  • The code is just an example. You don't need to be sarcastic. But thanks to reply! :D – David Rodrigues Jan 25 '16 at 18:28
  • Always remember, SRP is a mean to help your write code, it is not a rule to go by in every single case. Although not very popular as an answer, you may check my approach to SRP, what it means to me, what I believe is correct. Check the answer here. – Andy Jan 25 '16 at 18:48
  • 4
    The idea of "SRP" can be applied to packages, methods, classes, database tables, enums, ... pretty much to anything you design. SRP says that the thing (whatever it is) should have only one reason to change. It's a good rule of thumb if you are designing a large, complex body of software that almost certainly will have to be changed---probably many times over years or decades to come. On the other hand, you might not want to spend too much time worrying about it if you're writing a script that will be used for a day and then forgotten. – Solomon Slow Jan 25 '16 at 19:01
  • DestroyInstanceWhenNeeded() and BuildInstanceWhenNeeded() should be private, not public. Getting this right might make SRP in this example might make things more clear for you. – ravibhagw Jan 25 '16 at 21:24
6

Allow me to go Zen on you:

Please do not incur in analysis paralysis.

By that reasoning you could as well call your method destroyInstanceWhenNeededButOnlyIfYouCallMe().

A single line of common code between two methods, specially an instantiation is not an itch that needs to be scratched. SRP doesn't go into that minutiae. It's about classes doing too much or having more than one reason to change.

The original design is OK. The second design is too much thinking for so little gain.

3

Single Responsibility Principle is ultimately a design principle created to ensure that "A class should have only one reason to change". While this doesn't mean that it cannot apply to methods, it means that it is more strictly applicable to classes.

In determining the responsibility of your class or method, it is important to consider the consumer of your class or method. Who/what will use this class and what is its intended purpose? Is your class a business layer module designed to coordinate complex business tasks? is your class a persistence class designed to commit data to your persistence module? The consumers of these classes and their intended activities will often provide a hint as to whether or not your class is doing too much.

For that reason your example is not clear. Who will use Example? What does Example do? If you haven't defined what it really does and who/what it is for, how can you say if it does too much?

Drilling down into your specific methods, I believe you're starting to touch on issues other than Single Responsibility Principle. First off, your API is flawed as you expose all of your methods as public. I believe destroyInstanceWhenNeeded() and buildInstanceWhenNeed() should be private.

With that resolved you can consider your individual methods. Should buildInstanceWhenNeeded() exist? Maybe, but that's more of a Don't Repeat Yourself problem. Should destroyInstanceWhenNeeded() exist? That's subjective based on a number of factors. Does it improve the readability of rebuildInstance()? Does it self-document some non-obvious behaviours or algorithms required in rebuildInstance()?

As you can see there's much to consider here. To answer your questions:

  1. Should I apply SRP on methods, decoupling it, to make sure that it have a single responsibility on each method?

Subjectively yes if it actually improves readability of code, if the decoupled method encapsulates logic that doesn't directly relate to the method based on its signature name, or if it helps self-document logic that may otherwise need a comment

  1. If yes. Should I do it even when the method don't duplicate any effect? For instance: if rebuildInstance() doesn't exists here, then I don't need decouple getInstance() because is the unique instance that will, in fact, create a new instance of class. So in this case, do I still need create buildInstanceWhenNeed() or not?

Again, it's hard to say based on the issues with your example with regards to the consumer of your class, but I have given you reasons beyond duplication of effort for refactoring a method.

  1. If yes, but lower priority. And about performance? I mean, when it really affect the runtime, decreasing considerably the speed of application because of ultra decoupled methods (which perhaps only affects interpreted languages), but with high specifications to each one.

If your method calls are so specific that you're running into performance issues with the stack, you may have made a mistake along the way.

  • Thanks by reply! About "I believe destroyInstanceWhenNeeded() and buildInstanceWhenNeed() should be private." It is private, on really. – David Rodrigues Jan 25 '16 at 21:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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