tags:

views:

258

answers:

8

(this is a C-like environment) Say I have two instance objects, a car and a bodyShop. The car has a color iVar and corresponding accesors. The bodyShop has a method named "paintCar" that will take in a car object and change its color.

As far as implementation, in order to get the bodyShop to actually be able to change a car object's color, I see two ways to go about it.

  1. Use the "&" operator to pass in a pointer to the car. Then the bodyShop can either tell the car to perform some method that it has to change color, or it can use the car's accessors directly.

  2. Pass in the car object by value, do the same sort of thing to get the color changed, then have the method return a car object with a new color. Then assign the original car object to the new car object.

Option 1 seems more straightforward to me, but I'm wondering if it is in-line with OOP best practices. In general for "maximum OOP", is the "&" operator good or bad? Or, maybe I'm completely missing a better option that would make this super OOPer. Please advise :)

+3  A: 

I would assume that the responsibility of the bodyShop is to modify car objects, so #1 seems like the right way to go to me. I've never used a language where the "&" operator is necessary. Normally, my bodyShop object would call car.setColor(newColor) and that would be that. This way you don't have to worry about the rest of the original car's attributes, including persistence issues - you just leave them alone.

Scott Saunders
A: 

I too agree with the first 1. I can't say it's best practice because i'm never really sure what best practice is in other peoples minds... I can tell you that best practice in my mind is the most simple method that works for the job. I've also seen this aproach taken in the hunspell win api and other c-ish api's that i've had to use. So yea i agree with scott.

http://hunspell.sourceforge.net/

//just in-case your interested in looking at other peoples code

Dave
+2  A: 

Since you're interested in the best OOP practice, you should ignore the performance hit you get with option 2. The only things you should be interested in is do either option unnecessarily increase coupling between the two classes, is encapsulation violated and is identity preserved.

Given this, option 2 is less desirable since you can't determine which other objects are holding references to the original car or worse, contain the car. In short you violate the identity constraint since two objects in the system may have different ideas of the state of the car. You run the risk of making the overall system inconsistent.

Of-course your particular environment may avoid this but it certainly would be best practice to avoid it.

Last point, does your bodyShop object have state; behaviour and identity? I realise that you have explained only the minimum necessary but possibly the bodyShop isn't really an object.


Functional v OO approaches

As an interesting aside, option 2 would close to the approach in a functional programming environment - since state changes are not allowed, your only approach would be to create a new car if it's colour changed. That's not quite what you're suggesting but it's close.

That may sound like complete overkill but it does have some interesting implications for proving the correctness of the code and parallelism.

Chris McCauley
+1  A: 

Option 1 wins for me. The & operator is implicit in many OO languages (like Java, Python etc). You don't use "passing by value" in that languages often - only primitive types are passed in that way.

Option 2 comes with multiple problems: You might have a collection of cars, and some function unaware of it might send a car to bodyShop for painting, receive new car in return and don't update your collection of cars. See? And from more ideologic point of view - you don't create new object each time you want to modify it in real world - why should you do so in virtual one? This will lead to confusion, because it's just counterintuitive. :-)

Abgan
Matt B.
A: 

It depends on whether the body shop's method can fail and leave the car in an indeterminate state. In that case, you're better off operating on a copy of the car, or a copy of all relevant attributes of the car. Then, only when the operation succeeds, you copy those values to the car. So you end up assigning the new car to the old car within the body shop method. Doing this correctly is necessary for exception safety in C++, and can get nasty.

It's also possible and sometimes desirable to use the other pattern - returning a new object on modification. This is useful for interactive systems which require Undo/Redo, backtracking search, and for anything involving modelling how a system of objects evolves over time.

Pete Kirkham
+1  A: 

I am not sure what this "C-like environment" mean. In C, you need this:

int paintCar(const bodyShop_t *bs, car_t *car);

where you modify the contents pointed by car. For big struct in C, you should always pass the pointer, rather than the value to a function. So, use solution 1 (if by "&" you mean the C operator).

A: 

In addition to other optinions, option 1 lets paintCar method return a completion code that indicates if the car has changed the color successfully or there were problems with it

dmityugov
+5  A: 

Option 1 is prefered:

The bodyShop can either tell the car to perform some method that it has to change color, or it can use the car's accessors directly.

Even better still...create an IPaintable interface. Have Car implement IPaintable. Have BodyShop depend on IPaintable instead of Car. The benefits of this are:

  • Now BodyShop can paint anything that implements IPaintable (Cars, Boats, Planes, Scooters)
  • BodyShop is no longer tightly coupled to Car.
  • BodyShop has a more testable design.
+1 for testable design
swegi