vaibhav
vaibhav

Reputation: 52

Design decision : Adding new arguments to a method V/S creating an instance variable

Lets suppose we have a class Shape which has a method rotate(int velocity). This method makes a shape rotate with a speed of velocity(the parameter passed to rotate). This method has been called in a project, say at 100 places.

But now a new requirement comes, that the rotate functionality will also depend on the color of the shape, i.e. if the color is blue then the velocity should be decreased by 1, else no change should be made.

Inside the rotate method,

void rotate(int velocity, Color color) {
  if(color == blue)
      --velocity ;
}

Which of these would be the best possible solution? Or, does there exist a better solution? If yes, then what could it be?

Regards

Upvotes: 0

Views: 160

Answers (4)

Adriaan Koster
Adriaan Koster

Reputation: 16209

A good principle of OO is to co-locate related behavior and state. In this case the rotate behavior of shape depends on the colour state of shape, so it makes sense to co-locate both in the Shape class, c.q. create a field 'colour' and use it within the rotate method to customize the rotation behavior.

Apart from this design decision, you are really also asking about a refactoring decision: how do I handle the application code that depends on shape? My approach in cases like this is to think ahead: how many changes like these to the Shape class can we expect? If this is a rare change then you could just go ahead and change all the code locations that initialize the shape class so a colour is set. If shape changes more often, then you should be more rigorous and make your code less tightly coupled. A way to do that in this case is to create an abstract factory (or use the factory offered by a D.I. framework like Spring) so that the application code does not need to know the creation details of shape.

BTW your third option seems sub-optimal to me: part of the code is not made aware of the addition of the colour state to shape, and keeps calling the old 'deprecated' rotate method. This means that setting a shape's colour to blue will not universally affect the rotation behavior, but only in 'special cases'. This weakens the design and makes it harder for the developers after you to understand it.

Upvotes: 0

Sotomajor
Sotomajor

Reputation: 1386

As for me, I see classic example of inheritance usage here.

class Shape {
  public void rotate(int v) {}
}

class GreenShape extends Shape {
  public void rotate(int v){
    super.rotate(v + 10);
  }
}

Upvotes: 0

Swagatika
Swagatika

Reputation: 3436

  1. I think the first option is is tedious to implement. What if you miss at one place, what if later u realize that you need rotate(single parameter) again.
  2. The second option is irrelevant as many have already pointed out.
  3. 3rd I think is the best solution, as it will not break your code. You can have both the overloaded method, can use any of them as per requirement.

Upvotes: 0

Anthony Grist
Anthony Grist

Reputation: 38345

I'd say there are a couple of questions you need to ask yourself.

  1. Do you care about the color outside of the rotate method? If yes, make it an instance variable; if no, pass it to the rotate method.

  2. Are you likely to care about the color outside of the rotate method further down the line? If yes, make it an instance variable; if no, pass it to the rotate method.

  3. Are you always going to care about the color when calling the rotate method? If yes, make it an argument (to force them to set the color when rotating the shape).

Upvotes: 2

Related Questions