Posted this to pragprog list (Apr 2005):
So, if I'm coding up an abstract base class, or interface, and I've got some methods that are not common to all subclasses, but some, what should be done? (All this code is from memory, so it's probably got some wacky syntax in there).
class abstract Animal {
public abstract void sleep();
public abstract String noise();
}
class Dog extends Animal {
public void sleep() {
turnInCircles();
layDown();
shift();
fidget();
restHead();
closeEyes();
setConscious(false);
}
public String noise() {
return "bark!";
}
}
class Fish extends Animal {
public void sleep() {
/*
* For the sake of my example, fish don't sleep. But
* I guess they do:<a href=' http://www.newton.dep.anl.gov/askasci/bio99/bio99047.htm'> http://www.newton.dep.anl.gov/askasci/bio99/bio99047.htm</a>
*/
}
public String noise() {
/*
* Fish probably make some noise, too, but work with me here...
*/
return ""; // return null; better?
}
}
By making the methods abstract, I force classes that may not need to do anything to implement an empty method. I could do this:
So, if I'm coding up an abstract base class, or interface, and I've got some methods that are not common to all subclasses, but some, what should be done?
class Animal {
public void sleep() { }
public String noise() { return null; }
}
class Dog extends Animal {
public void sleep() {
turnInCircles();
layDown();
shift();
fidget();
restHead();
closeEyes();
setConscious(false);
}
public String noise() {
return "bark!";
}
}
class Fish extends Animal {
/*
* Now don't need to anything, just use the defaults in base class
*/
}
And this whole thing can get worse with several subclasses, and the base class (or interface) becomes the union of all methods.
Is it better to introduce intermediate classes/interfaces?
class Animal {
public void sleep() { }
}
interface NoisyAnimal {
public String noise();
}
interface SleepingAnimal? {
public void sleep();
}
class Dog extends Animal implements NoisyAnimal, SleepingAnimal {
public void sleep() {
turnInCircles();
layDown();
shift();
fidget();
restHead();
closeEyes();
setConscious(false);
}
public String noise() {
return "bark!";
}
}
class Fish extends Animal {
}
Now client code that has a list of animal instances has to do some additional checking of interfaces and type-casting to see if it can command its collection to all bark:
Animal<blockquote> zoo = new Animal[2] { new Dog(), new Fish() };
for (// each animal) {
Animal animal = zoo[i];
if (animal instanceof NoisyAnimal) {
((NoisyAnimal)animal).noise();
}
}
vs. the original way:
Animal<blockquote> zoo = new Animal[2] { new Dog(), new Fish() };
for (// each animal) {
Animal animal = zoo[i];
animal.noise();
}
Dave Thomas replies:
I guess this extract (your client code examples) contains the answer
to your question.
If we take it as a given that some animals don't make a noise., then
why would animal.noise() be expected to work for any arbitrary animal?
Why would you ever write code that iterates over a zoo and asks each
animal to make a noise? So I'm thinking the question isn't really about
the implementation of a class hierarchy, but rather about the _use_ of
the classes. I'm thinking that the design has made an incorrect
assumption, and that this assumption is then forcing you to do ugly
stuff to support it. It's an example of code pushing back.
Michael Feathers:
Whenever this sort of question comes up, there's usually a LiskovSubstitutionPrinciple (LSP) violation waiting in the wings. In general, if you subclass Animal and you have to ask an object whether it is a NoisyAnimal (or something else) before sending it a message, it's a bad use of inheritance. There are a couple of different references for LSP on the web:
http://www.objectmentor.com/resources/articles/lsp.pdf%20
http://javaboutique.internet.com/tutorials/JavaOO/
http://en.wikipedia.org/wiki/Liskov_substitution_principle
Michael Feathers
author, Working Effectively with Legacy Code
www.objectmentor.com
gedb@rushcoding.co.uk suggested the Decorator pattern for this.
Ron Jeffries:
Yes ... though I was taught that asking a class what it is before
sending it a message is not A Good Thing (tm, Martha Stewart).
That this might lead me to think that a good design might accept
that some animals make an empty noise will, I hope, be forgiven.
No, not a noise like a hollow drum. A noise of zero duration, zero
amplitude. No noise after all. But you all knew I meant that.
Dave later writes
You know, I can't agree with that in a static language such as Java,
because that's one step down the road to saying that “everything's an
object,” which we all know to be sinful.
Fundamentally, the problem is that class-based OO is flawed in its idea
that everything can be categorized hierarchically. The world just isn't
like that. Perpetuating the myth for the convenience of a compiler
seems to be a dangerous thing to do.
I come back to the question: given that you know there are animals that
cannot speak, why would you write code that asks them to? Or, putting
it another way: if you have a collection where objects share a
characteristic that you plan to employ (such as the ability to make a
sound), why would you fill it with mute beasts.
Clearly, the answer is convenience.
But such convenience isn't in the spirit of statically typed languages,
where employing null methods to bypass the type system is (to my mind)
an impedance mismatch. Having chosen a typing paradigm, it seems wrong
not to use it.
Cheers
Dave
Ron Jeffries chimes in:
My interests do not include making things convenient for compilers.
I'm interested in making things convenient for me, which I'll come
back to.
...
It would seem to me that in a system about animals, a collection of
animals isn't likely to want to be separated into mute and noisy
ones. I presume that the canonical collection of animals, the ones
on the Ark, were some noisy, some not. As Noah's jerk son Shem went
around harassing the animals, he probably noticed that some of them
were noisy and some quiet. But he probably mostly thought of them as
animals.
So the answer, for me, is in fact convenience ... in all its guises
such as code volume reduction, clarity, and the like. One of my key
measures of code goodness is lack of duplication.
So if the program includes the statement
if (animal instanceof NoisyAnimal) {
((NoisyAnimal)animal).noise();
once I don't like it, because it's complex and not all that clear.
If it's in there twice, I am duty-bound to remove it. There are lots
of ways. A very simple one is to implement noise() on all Animals,
and let some of them be very quiet. Roar, growl, peep, . C continuum, arguably. Quite possibly all the same to me.
So I'd not be standing on ceremony. Within reason, the simplest code
(in Beck's sense) is, for me, the best code. Not to steal your
thunder, but to me it's the pragmatic thing to do, in many cases.
How many? Well ... for me, darn near all. I can't think of a time
when I'd use instanceof without trying hard to get rid of it.
Could be a Smalltalk** thing. Or just another screwed-up Jeffries
thing.
** This smalltalk tidbit from c2:
“In Smalltalk, subclasses are frequently not subtypes because of the overriding of superclass methods with self messageNotUnderstood. This is incorrect design from an object purism point of view, but it has made such things as the Smalltalk-80 class hierarchy efficient and minimal. The lesson to learn is that striving
for object purism is ridiculous. -- SunirShah?” -> c2
DaveThomas replies to Ron:
That's exactly my point. The need to have this kind of taxonomy is a
part of the weakness of conventional, class-based OO.
The options are:
1. Have a ridiculously complex, mutli-dimensional hierarchy in an
attempt to model all possible combinations of characteristics. There
seems to be agreement that this is silly.
2. Use something such as instanceOf to test. This implies to some
extent that we've done at least part of (1), and therefore shares some
of the problems. It's also remarkably coupled, and hence is a bad idea
all around.
3. Use a modified form of (2), where you do something like
if (animal.can_make_noise())
animal.talk();
This seems a tad better, because you make the test a responsibility
of the object, rather than the caller.
4. Have the base class fake out every single potential attribute of all
animals, and then override these null methods in subclasses. This seems
like a large amount of coupling to me.
So none of these approaches is particularly good. Which leads me to a
conclusion. Doctor, it hurts when I do this...
Why do it at all? It's a design smell...
Cheers
Dave
Ron
Interesting ... but are you suggesting that NO animal can make
noise? Is there a “solution” that seems better ... in general, or in
particular?
So far, I like empty noise, but I'm a heathen, as you know.
Dave
No, I'm saying two things:
1. Animal is not necessarily a class
2. It's a smell to write code that asks objects to do things when you
know that the population being asked can't necessarily do them.
~~~
Michael Feathers elsewhere in the big thread:
It all seems so intractable when we use the animal example too.
Here's what I always fall back on: objects are not there to model things
in the real world, they are there to perform some role, and they are a
locus of responsibility. When we tangle responsibilities together in
the same class, we end up having grief, so it is better to separate
responsibilities into separate classes so that they can vary
independently. Sometimes we need a null object, and that's okay.
I really dislike the animal example because it fails abjectly on the
first part of that: objects are not there to model the world.
Michael Feathers (the day I need a cow class is the day I...)
As I study this, here's where I've left off:
Given these two options:
Animal[ ] zoo = new Animal[2] { new Dog(), new Fish() };
for (// each animal) {
Animal animal = zoo[i];
if (animal instanceof NoisyAnimal) {
((NoisyAnimal)animal).noise(); // only Dog implements NoisyAnimal
}
}
Animal[ ] zoo = new Animal[2] { new Dog(), new Fish() };
for (// each animal) {
Animal animal = zoo[i];
animal.noise(); // Fish just silently does nothing
}
instanceof is a design smell for Ron Jeffries.
instanceof is a smell of LSP violation for Michael Feathers.
instanceof can lead to class explosion? (lots of little interfaces everywhere.)
silent implementor leads to massive, union interface
silent implementor is common in Smalltalk? And Ron is fine with it.
Seems both are smells to Dave, and says don't do either. He confuses me the most (which is usually the storm before the calm, with me and him).
1. Animal is not a class (which matches one option for resolving Circle/Ellipse?)
2. Silent implementor is a smell, because the client is forcing it in order to have the convenience of not knowing?
3. InstanceOf smells because it means we may have class explosion going on, and is remarkably coupled (client code coupled to many subclass instances, when the client is supposed to be protected from that).
tags: ComputersAndTechnology
