Writing Single responsibility methods [closed]

Question

Let's say we have a typical service class method who does few things, input validation, db object retrieval, some business logic execution and then saving and returning record to user. This is irrespective of any language we are using.

methodA(args1, args2, args3) {

    returnValue1 = validateArgs(args1, args2, args3)
    returnValue2 = dbObjectRetrieval(args1)
    businessLogicValidatorMethod(returnValue2, args1, args2, args3)
    enrichArgs(args1, args2, args3)
    returnValue3 = saveArgs(args1, args2, args3)

    return returnValue3;
}

Now when we say that one method should only do one thing, the above method is clearly doing more than one things, Needless to say all the business logic that goes into the picture. I can understand that for methods like validateArgs, dbObjectRetrieval, businessLogicValidatorMethod it is very easy to do one thing, in the sense

a. validateArgs should not go to db, instead all the required data should be passed and method argument except some environment configurations may be.

b. dbObjectRetrieval should not try to validate anything and either returns the data or throws exceptions (lets say in the case of not found)

similarly for other methods.

This will have obvious benefit that these single responsibilities function (where there are not side effect of getting input form somewhere else but inputs are passed as params) can be reused across the application.

But What if some functionality of methodA is duplicated in methodB. should we move that logic into another methodC and both methodA and methodB start calling methodC? How do you make sure that one function can not be broken down any further?

Answer 1

First, methodA is doing one thing in software term : calling required methods and return. It's a good facade method example. More than one line of codes doesn't mean it is doing more than one thing.

But What if some functionality of methodA is duplicated in methodB. should we move that logic into another methodC and both methodA and methodB start calling methodC? How do you make sure that one function can not be broken down any further?

Duplicated methods or codes you wrote more than once are about "Don't Repeat Yourself(DRY)". It is not about Single Responsibility. You can split methods and/or define private methods that can be called from other methods to achieve DRY and it's totally about your code. You can change it by new requirements.

methodA(args1, args2, args3) {

    returnValue1 = validateArgs(args1, args2, args3)
    returnValue2 = dbObjectRetrieval(returnValue1)
    businessLogicValidatorMethod(returnValue2, args1, args2, args3)

    return methodC(args1, args2, args3);
}

methodB(args1, args2, args3) {

    anotherBusinessLogicValidatorMethod(args1, args2, args3)
    return methodC(args1, args2, args3);
}

methodC(args1, args2, args3) {
    enrichArgs(args1, args2, args3)
    returnValue3 = saveArgs(args1, args2, args3)

    return returnValue3;
}

Answer 2

One thing, is not the same as one line of code. I actually like to have a look at the naming, when making these decissions. If I can make a (somewhat) short and precise name, which describes everything that is going on in the method, then I am happy. Something like CalculateYearlyProfit() would be good. Even though this might actually do mulitple things (maybe it needs to calculate the profit for each month first). However, CalculateYearlyProfitAndValidateThatNumberIsNotNegative() makes me unhappy.

I know this might not be the exact answer that you are looking for, but naming can be quite helpful if done right. It can also be quite difficult, as stated in the famous quite:

There are only two hard things in Computer Science: cache invalidation and naming things.

But once you get it right, it is deffinitely worth the effort. So to sum up, ask yourself the following two questions:

• Does my method name describe everything the method does, in a precise manner?
• Does my method name cover one, and only one, responsiblity?

If you can answer "Yes" to both, then I would say you are off to a good start.

Answer 3

To start off, methodA also looks to have just a single responsibility, the responsibility of orchestrating that a piece of business logic can do its work.

But What if some functionality of methodA is duplicated in methodB. should we move that logic into another methodC and both methodA and methodB start calling methodC? How do you make sure that one function can not be broken down any further?

The question is how much code was duplicated and if it is a real duplication or just very similar code.

If the duplicated code is just a single line, then there is clearly no point in extracting that to a new function to reduce duplication. Even if it is 2 or 3 lines, I would be hesitant to put that in a separate function, unless I can come up fairly easily with a good descriptive name for that function.

What is even more important is the determination if the code is really a duplicate or if two distinct pieces of code just happen to look the same. This takes a bit of experience, but the question you have to ask yourself is: If a bug gets reported for which the root-cause lies in this piece of apparently duplicated code in methodA, do I then have to make the exact same change to the code of methodB as well to remove the bug completely from the entire system. If the answer is yes, then the code is really duplicated and it will improve the maintenance effort if it is extracted to a function of its own.