CODEWITHSUNDEEP
oopdesign-patternssingle-responsibility-principle
user437291
user437291

Reputation: 4651

Is Car class violating Single Responsibility Principle?

Even thought I think I understand Single Responsibility Principle and high/low cohesion principle, the following questions are still causing me some confusion

1) Assume Planet and Bird properties are put arbitrarily / at random in Car class ( ie. no code within Car needs or operates on the two objects returned by these two properties ) - in other words, Planet and Bird properties don't belong in Car class

a)

SRP states that object should have only one reason to change.

public class Car
{
    public void StartEngine()
    { ... }

    private Planet Planet
    {
        get { ... }
    }

    private Bird Bird
    {
        get { ... }
    }
}

Is Car class violating SRP? I would say it doesn't break SRP, since any changes to Planet or Bird instances don't propagate to the Car class?

b)

Cohesion refers to how closely related methods and class level variables are in a class. In highly cohesive class all the methods and class level variables are used together to accomplish a specific task. In a class with low cohesion functions are randomly inserted into a class and used to accomplish a variety of different tasks

Assume that even though Car class contains these two random properties, it still accomplishes just a single specific task ( or several closely related task ):

Would we say that Car has low cohesion, even though it still performs a specific task ( or several closely related tasks )?

2) Assume that Planet and Bird properties are used by methods of a Car instance to accomplish a specific task, would then Car have high cohesion, even though conceptually the two properties don't belong to Car ( and thus it would be better if instead Planet and Bird instances were passed as arguments to a methods of a Car which operate on them )

thank you

HELTONBIKER:

1)

as you encapsulated Bird and Planet inside Car (worse yet if they are private), so now Car class has THREE reasons to change:

I fail to see how Car has three reasons to change since in my first question Car's methods don't even operate on the two properties and thus any changes to Planet's and Bird's public API won't affect Car class?

2)

The problem here has two components:
1.  Bird and Planet are contained (as opposed to aggregated) in Car class;
2.  Bird and Planet are not conceptually related to Car, much less by some containment relationship.

a) This is confusing: aren't the chances ( at least with my first question ) of Car having to be modified due to modification of Planet or Bird instances exactly the same regardless of whether Planet and Bird instances are contained or aggregated?

b) In second question methods of Car do operate on the two properties in order to perform a single specific task, so aren't they conceptually at least somewhat related? Would you say that even in second question class has low cohesion, even though it performs only a single task ( and is using the two properties to accomplish the task )?

Upvotes: 1

Views: 1727

Answers (3)

heltonbiker
heltonbiker

Reputation: 27575

SRP means that a class should have only one reason to change.

So, a modification in Car class should mean that the Car conceptual model changed.

BUT, as you encapsulated Bird and Planet inside Car (worse yet if they are private), so now Car class has THREE reasons to change:

  1. Modifying the Car conceptual model and/or behaviour;
  2. Modifying the Bird conceptual model and/or behaviour;
  3. Modifying the Planet conceptual model and/or behaviour;

The problem here has two components:

  1. Bird and Planet are contained (as opposed to aggregated) in Car class;
  2. Bird and Planet are not conceptually related to Car, much less by some containment relationship.

Or, plainly speaking (and I hope you did so as a didactic exercise), the architecture shown simply doesn't make sense.

Example with aggregation (in Python). The non-cohesive classes are defined outside the Car class definition, which references them. Car depends from Bird and Planet, but now Bird and Planet exist on their own.

class Bird():
    hasWings = True

class Planet():
    isFlat = False

class Car():
    owner = Bird()
    substrate = Planet()

Example with parameter-passing (just the car class, suppose the other classes are similar as above). Now the Car constructor (the __init__ method in python) takes instances as parameters. This might or might not be prefered. The dependency and coupling remains, but perhaps more concrete now.

class Car():
    def __init__(bird, planet)
        owner = bird
        substrate = planet

In the end this whole issue of cohesion and coupling doesn't have so much to do with the software itself, but with the developers. Compilers won't mind if your namespaces, project folders and file distribution is messy, as long as it "compiles". But it wouldn't make ANY SENSE to do as you did (put a Bird and a Planet class inside a Car class). Just to begin, your versioning of each class would be very messed.

So, the purity you shouldn't violate is not that written in books for the sake of it. This purity is (or should have been) derived of human beings struggling with machine instructions. Object-Orientation, and Software Architecture in general, is not intended for the machine, but for the developer's (piece of) mind.

Upvotes: 1

JerKimball
JerKimball

Reputation: 16894

The car class does have low cohesion, as it refers to classes wholly dissimilar to it's set of responsibilities. It also has a higher coupling surface, because since Planet and Bird are public, you've provided access to consumers to these properties, meaning that you're now adding two more "reasons for change" to any consumer, regardless of whether or not Car uses these internally.

At any rate, SRP has been violated if only because Car now has the responsibility of "a way to get planets or birds", disregarding any coupling/cohesion arguments.

Upvotes: 1

fen
fen

Reputation: 10105

1) I would say that Car cannot hold Planet and Bird. That way Car has two different responsibilities: car functionality and holding some strange objects. There should be some other object/class that would hold objects in world: eg: class WorldContainer

2) I would say that both of your examples have low cohesion. Managing car and some different objects should be done using some other interface. Interface that would glue them together.

Upvotes: 1

Related Questions