Which SOLID Principles are violated?

Question

INTRODUCTION

---

I work on my master thesis about inheritance problems and work out some indicators which show that an inheritance problem exist.

Like the following example:

EXAMPLE

---

public static String getAnimalNoise(Animal animal) {
  if (animal instanceof Dog)
    return "Woof";
  if (animal instanceof Cat)
    return "Miau";
  return "";
}

The method returns the String "Woof" if the given Animal instance is a Dog and "Miau" if it is a Cat. The empty string because some animals make no noises at all.

So the correct solution for that should be use polymorphism with a getNoise Method in the Animal class.

I have analysed different indicators of inheritance problems and want to say if some of them violates a SOLID Principle.

I thought the example above violates the:

1. Single responsibility principle (SRP)
2. Open/closed principle (OCP)
3. Liskov substitution principle (LSP)
4. Dependency inversion principle (DIP)

But i'm not really sure whether it is true for all.

I thought:

PRINCIPLE VIOLATIONS

---

SRP Violation

Because conditional statements at all violates the SRP, because like the switch case statement or more than one if-else statement are consider more than one responsibility.

It exists two cases so there are more than one reason to change the method.

OCP Violation

Because if a new animal is added a new case must be added to the method so the Method is not close for modifications.

LSP VIOLATION

Each branch executes different actions dependent of the animal sub type. Which i think violates the LSP ?! I know the example of the rectangle and square and the getArea but these example i thought fits also to the violation.

DIP VIOLATION

The conditional statements take dependency that means the statements are dependent on details and not on abstractions which violates the DIP.

QUESTION:

---

So the Question is, for the given example, are the given principles really violated and is the reasoning correct?

Olvan

This might be more on-topic on Programmers.SE. – Transferor

Answer 1

SRP Because conditional statements at all violates the SRP, because like the switch case statement or more than one if-else statement are consider more than one responsibility. It exists two cases so there are more than one reason to change the method.

I strongly disagree. The SRP is meant to be interpreted with a pinch of salt. Read Uncle Bob's article on it here - he coined this principle.

I'll quote the important bits:

What defines a reason to change?

This principle is about people.

When you write a software module, you want to make sure that when changes are requested, those changes can only originate from a single person, or rather, a single tightly coupled group of people representing a single narrowly defined business function. You want to isolate your modules from the complexities of the organization as a whole, and design your systems such that each module is responsible (responds to) the needs of just that one business function.

[...] as you think about this principle, remember that the reasons for change are people. It is people who request changes. And you don't want to confuse those people, or yourself, by mixing together the code that many different people care about for different reasons.

---

OCP Because if a new animal is added a new case must be added to the method so the Method is not close for modifications.

Correct. The method assumes a specific set of implementations, and would not be able to handle new ones without being modified.

---

LSP Each branch executes different actions dependent of the animal sub type. Which i think violates the LSP ?!

It violates the LSP, but for a different reason. If i were to pass in a giraffe, I would get an unexpected result, an empty string. Which means the method is not correct for any subtype of Animal.

---

DIP The conditional statements take dependency that means the statements are dependent on details and not on abstractions which violates the DIP.

Technically true, but this is just a side-effect of violating the other two principles above. It's not really the core of the problem.

---

Remember that principles are not rules, so don't be too strict/literal when interpreting them. Pragmatism and understanding why a principle is needed are key.

Breakable

I'd add the DIP: the method should depend on a MakesNoiseInterface instead of concrete classes. – Substratum

@Substratum No need for an interface, an abstraction already exists: Animal. The problem is the type checking against specific animals, and I would personally solve it by moving the getNoise method inside Animal, solving all 3 violations. – Breakable

Thanks a lot for the fast answer! For SRP i think you are wrong see the the Book of Robert C. Martin Clean Code site 38 >There are several problems with this function. First, it’s large, and when new employee types are added, it will grow. Second, it very clearly does more than one thing. Third, it violates the Single Responsibility Principle7 (SRP) because there is more than one reason for it to change. Here the clean code reference: cleansourcecode.files.wordpress.com/2013/10/clean-code.pdf – Olvan

@Olvan Robert C. Martin is Uncle Bob, whose article I mentioned ^^ I don't think there's more than one reason to change. Keeping the article in mind, can you name two or more reasons for it to change, coming from different people from different departments/different business requirements? – Breakable

I know he is. Say you want to change the noise of a dog one reason for me, second reason is the noise of the cat to change and thirds the reason that an animal should return not an empty string for the noise something like that. – Olvan

Changing the noise of a dog, or a cat, or any other animal are one and the same reason. They're the same business requirement. "(..) remember that the reasons for change are people. It is people who request changes." The person who cares about the noise a cat makes is also the person who cares about the noise a dog makes. – Breakable

Ok perhaps you're right i think the example is also inappropriate chosed – Olvan

"I would personally solve it by moving the getNoise method inside Animal" – that's essentially what I meant to express with the above comment. The functionality should be expressed with an interface [which implicitly meant the object itself had a method you could call]. – Substratum

@Substratum Then we're on the same page :) – Breakable

Answer 2

I disagree to the fact that your example code violates the LSP. LSP is defined as followed:

Let Φ(x) be a property provable about objects x of type T. Then Φ(y) should be true for objects y of type S where S is a subtype of T.

Given the two derivation from Animal, namely Dog and Cat and your method getAnimalNoise you are not making decisions about some proveable property of the concrete object. Your method is deciding what noise should be returned and not the objects by there self.

So lets imagine you can set the number of legs for an animal.

Animal a = new Animal()
a.setLegFront(2);
a.setLegRear(2);

And if your Dog overwrites this like this:

 class Dog extends Animal
 {
   public void setFrontLegs(int legs)
   {
     this.frontLegs = legs;
     this.rearLegs = legs + 2; 
   }
   public void setRearLegs(int legs)
   {
     // do nothing here for demonstration purposes
   }
 }

If you now have a factory returning a Animal

Animal createAnimal()
{
   return new Dog();
}
Animal a = createAnimal();
a.setFrontLegs(2);
a.setRearLegs(2);

And you call the methods setFront/setRearLegs you are expecting the results to be 2 and 2 but in fact the result is completely different, namely 2 and 4. So this example is tightly bound to the common LSP example with squares and rectangles. But for me its a more accurate example of a violation of provable properties than in you example.

Update for the other principles

I agree with you that the other principles are also violated but for SRP I agree with @dcastro.

Inhabited

Thanks. Whats about the other principles? – Olvan