Is adding public methods to base classes bad design?

A

Andrew Ward

Hi All,
Sorry if this is off topic, but I could not seem to find a suitable OO
Design newsgroup. If there is one feel free to let me know.

Here is a simplification of a general design problem I face.

Say you have:

class Car {
virtual void speedup();
virtual void slowdown();
};

class BondCar {
virtual void self_destruct();
};

Now storing a std::list<Car *> and calling speedup/slowdown is fine, however
if you wanted to self_destruct all BondCars in the list I can think of a few
ways to do it:

1) Use dynamic_cast, this is obviously not a good solution for more complex
designs.
2) Bring self_destruct down into the base class, making a 'fat' interface
3) Storing two lists, one std::list<Car *> and the other std::list<BondCar
*>, but then if I want to speed up all cars I have to iterate over two lists

None of these solutions seems very well suited to more complex designs, and
I have come across this problem over and over again. As it says in the c++
faq lite, inheritance is for old code to call new code, and NOT for code
reuse, so it almost seems to me that adding new public methods to derived
classes is wrong.

Could someone please enlighten me?
Andy
 
D

David White

Andrew Ward said:
Hi All,
Sorry if this is off topic, but I could not seem to find a suitable OO
Design newsgroup. If there is one feel free to let me know.
comp.object

Here is a simplification of a general design problem I face.

Say you have:

class Car {
virtual void speedup();
virtual void slowdown();
};

class BondCar {
virtual void self_destruct();
};

Now storing a std::list<Car *> and calling speedup/slowdown is fine,

No it isn't. Those members are private.
however
if you wanted to self_destruct all BondCars in the list I can think of a few
ways to do it:

1) Use dynamic_cast, this is obviously not a good solution for more complex
designs.

I always wince at having any dynamic_cast, but it's still the best of a bad
choice of solutions in some cases.
2) Bring self_destruct down into the base class, making a 'fat' interface

If 'self_destruct' makes sense for all Cars, then it should be in Car. If it
only makes sense for BondCars, then it should only be in BondCar. I don't
think its location should be dictated by your application without regard for
its proper place. I don't have a problem with "fat" interfaces. In fact, the
more abstract the class in which you can put a member, the better, as long
as the member makes sense in that class.
3) Storing two lists, one std::list<Car *> and the other std::list<BondCar
*>, but then if I want to speed up all cars I have to iterate over two
lists

A variation on 3) is to have one list with all Cars, including BondCars, and
another with just BondCars. It's harder to delete a BondCar, as it has to be
removed from both lists, but you don't need any downcasts and you don't need
to put members in the wrong class.

It's impossible to choose without knowing your application better.
None of these solutions seems very well suited to more complex designs, and
I have come across this problem over and over again. As it says in the c++
faq lite, inheritance is for old code to call new code, and NOT for code
reuse, so it almost seems to me that adding new public methods to derived
classes is wrong.

I thought it was to represent IS-A relationships, but maybe I'm behind the
times.

There's nothing wrong with adding anything to anything as long as it solves
your problem, makes sense, and doesn't cause a maintenance nightmare.
Could someone please enlighten me?

That's about the best I can do.

DW
 
J

jeffc

Andrew Ward said:
Hi All,
Sorry if this is off topic, but I could not seem to find a suitable OO
Design newsgroup. If there is one feel free to let me know.

Here is a simplification of a general design problem I face.

Say you have:

class Car {
virtual void speedup();
virtual void slowdown();
};

class BondCar {
virtual void self_destruct();
};

I'm assuming those are supposed to be public functions.
Now storing a std::list<Car *> and calling speedup/slowdown is fine, however
if you wanted to self_destruct all BondCars in the list I can think of a few
ways to do it:

First, I think you should take a step back and ask yourself about the
implications of a design where you would even consider having a list of base
class objects, and then attempting to do subclass functions on only some of
those objects. This isn't a trivial subject.
None of these solutions seems very well suited to more complex designs, and
I have come across this problem over and over again. As it says in the c++
faq lite, inheritance is for old code to call new code, and NOT for code
reuse, so it almost seems to me that adding new public methods to derived
classes is wrong.

Generally speaking, I can't imagine why public methods in subclasses could
be wrong. Inheritance is used often, even when there is no such thing as
"old" code and "new" code. i.e. it's used right in the original design.
Anyway, I don't think you have a fundamental OO problem with inheritance, I
think you have a specific design issue related to lists of things. You can
do more reading in this area. The C++ FAQs book by Cline is a short start
(is bag-of-apple a kind-of bag-of-fruit?, etc.)
 
E

ES Kim

Andrew Ward said:
Hi All,
Sorry if this is off topic, but I could not seem to find a suitable OO
Design newsgroup. If there is one feel free to let me know.

Here is a simplification of a general design problem I face.

Say you have:

class Car {
virtual void speedup();
virtual void slowdown();
};

class BondCar {
virtual void self_destruct();
};

Now storing a std::list<Car *> and calling speedup/slowdown is fine, however
if you wanted to self_destruct all BondCars in the list I can think of a few
ways to do it:

1) Use dynamic_cast, this is obviously not a good solution for more complex
designs.
2) Bring self_destruct down into the base class, making a 'fat' interface
3) Storing two lists, one std::list<Car *> and the other std::list<BondCar
*>, but then if I want to speed up all cars I have to iterate over two lists

None of these solutions seems very well suited to more complex designs, and
I have come across this problem over and over again. As it says in the c++
faq lite, inheritance is for old code to call new code, and NOT for code
reuse, so it almost seems to me that adding new public methods to derived
classes is wrong.

Could someone please enlighten me?
Andy

I've encountered a similar situation where self_destruct() makes sense only
for BondCar. I'm considering introduction of a member which tells me to what
class a specific object belongs. Then I can check the member before applying
self_destruct() to an object. It's not the OO way, my code would look ugly,
but I expect it'll work. Any suggestion please?
 
J

John Harrison

ES Kim said:
I've encountered a similar situation where self_destruct() makes sense only
for BondCar. I'm considering introduction of a member which tells me to what
class a specific object belongs. Then I can check the member before applying
self_destruct() to an object. It's not the OO way, my code would look ugly,
but I expect it'll work. Any suggestion please?

It would be better to use dynamic_cast.

john
 
D

David White

ak said:
|1) Use dynamic_cast, this is obviously not a good solution for more complex
|designs.
|2) Bring self_destruct down into the base class, making a 'fat' interface
|3) Storing two lists, one std::list<Car *> and the other std::list<BondCar
|*>, but then if I want to speed up all cars I have to iterate over two lists
|
|None of these solutions seems very well suited to more complex designs, and
|I have come across this problem over and over again. As it says in the c++
|faq lite, inheritance is for old code to call new code, and NOT for code
|reuse, so it almost seems to me that adding new public methods to derived
|classes is wrong.


I would go for (2), having a virtual self_destruct() in base class which does
nothing but implemented in the derived class.

So where you have:
pCar->self_destruct();

the car will refuse unless it's a BondCar? A self-destruct that leaves the
car intact.

DW
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,763
Messages
2,569,562
Members
45,038
Latest member
OrderProperKetocapsules

Latest Threads

Top