4

I was recently assigned to design a class which contained business logic in a single public method

 class MyClass
       private BusinessObject object;
       public BigInteger calculateCost() {
              //do calcualation using properties of object
       }
 }

Calculation done in method calculateCost() is perfectly alright however there are other properties of object which can change the way calculation is done. So based on some condition I should be able to apply discount, there are multiple conditions each can change the calculation is done.

So I applied the simple approach by creating private methods like below

     private calculateCost1() {
           //using object's properties calculate the cost
     }

     private calcualteCost2() {
           //using object's properties calculate the cost
     }

And called these methods from the public method

      public BigInteger calculateCost() {
              //do calcualation using properties of object
              calculateCost1();
              calculateCost2();
      }

Cons of this design is that if I need to add extra method of calculation, I will have to change the MyClass however I got the feedback that its not following Single Responsibility Principle. I believe the class's single responsibility is to calculate the cost and after adding extra methods to calculate the cost different way based on business object's properties, it's still adhering to SRP.

Can anyone please comment why this design is not following SRP if it's not really?

Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
Eager
  • 225
  • 1
  • 8
  • Without further information, I think it adheres to SRP – fps Feb 14 '17 at 14:09
  • Are you also responsible for designing the `BusinessObject` class, or is it given? – Calculator Feb 14 '17 at 14:10
  • Or you do it there, or in other classes and call those classes from calculate cost. Either way the final value needs to be changed before it is returned. – bichito Feb 14 '17 at 14:12
  • If you want to keep it simpler you can keep a list of deductions (%) loop through them before returning the final value. No extra classes or further modifications – bichito Feb 14 '17 at 14:29

1 Answers1

5

I got the feedback that its not following Single Responsibility Principle

Your class is following the Single Responsibility Principle since all the methods in the class have a single objective of calcualting cost.

What your class is not adheering to is the Open-Closed Principle. You need to modify your class every time you need to introduce a new calculation mechanism. The Open Closed Principle states that classes should be open to extension but closed to modificaiton. One of the ways you can adhere to the OCP in your case is to use the Strategy Pattern where you have one class for each calculation type.

Community
  • 1
  • 1
Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82