method is not designed for extension

[Efi Merdler]

Hi,
I started using CheckStyle in my projects and one of the erros that I
encountered was
"method is not designed for extension" i.e. method declaration should
be either empty, final or abstract.

I remember reading about this subject that sub classing a class that
was not supposed to be sub classes is a dangerous thing.
My question is how you as a developer knows when your class will be
subclasses, that is when you write your classes you never know how
people will use them in the future, so preventing people from sub
classing is counter productive.

Thanks,
Efi

 

[Lew]

Hi,
I started using CheckStyle in my projects and one of the erros that I
encountered was
"method is not designed for extension" i.e. method declaration should
be either empty, final or abstract.

Those are three very different things. So the "i.e. [sic]" makes no sense.
Final and abstract methods are pretty much the opposite of each other.
Abstract certainly means "designed for extension", in fact, "required to be
extended". Final means not allowed to be extended, the antithesis of abstract.
One might say "final" means "designed not to be extended". Empty merely means
no action occurs in the body, and can be either final or not.

What does Checkstyle's documentation say about this particular message?
Certainly not that the method "should be either empty, final or abstract".

I remember reading about this subject that sub classing a class that
was not supposed to be sub classes is a dangerous thing.

Turn that on its head. Allowing a class to be subclassed without taking
certain precautions is a dangerous thing.

Furthermore, even when the mechanics of extension are correct, there is a
modeling issue. Inheritance models an "is-a" relation: an object of the child
type "is-a" thing of the parent type also. All too often, people create
inheritance hierarchies where the model does not call for an "is-a" relation.

Joshua Bloch covers these topics admirably in /Effective Java/.

My question is how you as a developer knows when your class will be
subclasses, that is when you write your classes you never know how
people will use them in the future, so preventing people from sub
classing is counter productive.

Au contraire! Preventing a class from being inherited is highly productive, if
the model calls for it. A developer does know certain things about how people
will use their API - all those things that the developer was careful to ensure
in the API "contracts", i.e., the method signatures. So, for example, if you
declare a class final then you as the developer do know, for a fact, that no
one will subclass it. If you declare a non-final class method to be final,
then you as the developer know, for a fact, that no subclass of your class
will override that method. These can be very useful restrictions to place on
clients of your API.

The secret is that the developer knows exactly how their API will be used,
because the developer is the very one who defines how their API will be used.
What the developer cannot know, or needs to care about, is where their API
will be used.

A subtle but significant point.

Google "design by contract".

- Lew

 

[Chris Uppal]

I started using CheckStyle in my projects and one of the erros that I
encountered was
"method is not designed for extension" i.e. method declaration should
be either empty, final or abstract.

I remember reading about this subject that sub classing a class that
was not supposed to be sub classes is a dangerous thing.
My question is how you as a developer knows when your class will be
subclasses, that is when you write your classes you never know how
people will use them in the future, so preventing people from sub
classing is counter productive.

The message relates to a certain way of designing classes so that subclassing
is safe. The overwhelming concern (to proponents of this approach) is
/safety/. So, if you create a class, you can ensure that it is safe by either
(a) making it final, thus ruling subclassing out of the equation, or (b) by
locking down all the unsafe bits.

The way to do (b) is to decide (as part of your design) /what/ kinds of
subclassing are to be permitted, and then to split your methods up into two
categories. One category is all the code which forms the heart of your class,
and which you cannot guarantee will be safe to override (because it is
necessary to the correct operation). That group of methods you make final (or
private). The other group of methods are /specifically designed/ to be
overridden, they are "hooks" where subclasses can substitute their own code.
These you do not make final (obviously!) but it isn't safe to have non-trivial
code in them, since the subclass override may call them before it's own code,
after it's own code, in the middle of it's own code, not at all, or many times
over -- you (the creator of the base class) have no control over that.

So you end up with methods all being either final or trivial or abstract.

Now, I think that approach is overdone myself, but there is no getting away
from the fact that it is the only way to ensure complete safety. If you don't
follow it then a subclass can break a superclass implementation without
intending to, and (much worse) a small change to an existing superclass can
break existing subclasses (called the "fragile base-class" problem). The
message from CheckStyle is telling you that you are not following that
approach.

-- chris

 

[Efi Merdler]

The message relates to a certain way of designing classes so that subclassing
is safe. The overwhelming concern (to proponents of this approach) is
/safety/. So, if you create a class, you can ensure that it is safe by either
(a) making it final, thus ruling subclassing out of the equation, or (b) by
locking down all the unsafe bits.

The way to do (b) is to decide (as part of your design) /what/ kinds of
subclassing are to be permitted, and then to split your methods up into two
categories. One category is all the code which forms the heart of your class,
and which you cannot guarantee will be safe to override (because it is
necessary to the correct operation). That group of methods you make final (or
private). The other group of methods are /specifically designed/ to be
overridden, they are "hooks" where subclasses can substitute their own code.
These you do not make final (obviously!) but it isn't safe to have non-trivial
code in them, since the subclass override may call them before it's own code,
after it's own code, in the middle of it's own code, not at all, or many times
over -- you (the creator of the base class) have no control over that.

So you end up with methods all being either final or trivial or abstract.

Now, I think that approach is overdone myself, but there is no getting away
from the fact that it is the only way to ensure complete safety. If you don't
follow it then a subclass can break a superclass implementation without
intending to, and (much worse) a small change to an existing superclass can
break existing subclasses (called the "fragile base-class" problem). The
message from CheckStyle is telling you that you are not following that
approach.

-- chris

I see, it reminds the difference between C++ and java, in c++ methods
are not virtual by default, in order to make them virtual you have to
define them like that, wheres in java methods are virtual by default.

I remember reading in Mayer's book (http://archive.eiffel.com/doc/
oosc/) about this discussion, he said (if I remember it correctly)
that you as a designer never know how your class will be used in the
future therefore the best solution is make all your methods virtual
(back then it was a question of performance).

Thanks chris,
Efi

 

[Lew]

There are ways to layer additional safety where you can't or won't do one of
those three. One simple one is to use a final method that throws
IllegalStateException if a subclass somehow subverted what a superclass would
have done. This would be detected on some subsequent operation when the state
is needed, and the exception thrown.

Not perfect, but it allows a little latitude in making non-trivial overridable
methods. It is also useful when you need to call a setter after construction
before some other action. The other action uses IllegalStateException to
signal that the setter was not called.

- Lew

 

[Chris Uppal]

Efi Merdler wrote:

[me:]

Now, I think that approach is overdone myself, but there is no getting
away from the fact that it is the only way to ensure complete safety.
If you don't follow it then a subclass can break a superclass
implementation without intending to, and (much worse) a small change to
an existing superclass can break existing subclasses (called the
"fragile base-class" problem). The message from CheckStyle is telling
you that you are not following that approach.

[...]
I remember reading in Mayer's book (http://archive.eiffel.com/doc/
oosc/) about this discussion, he said (if I remember it correctly)
that you as a designer never know how your class will be used in the
future therefore the best solution is make all your methods virtual
(back then it was a question of performance).

I think it depends on what you are trying to maximize. If it's safety then
only trivial (empty or abstract) methods should be virtual; if it's flexibility
then almost everything should be virtual.

It seems odd that Meyer (usually a safety fanatic) should advocate the "unsafe"
approach -- I suspect he may have been reacting to the once-common
(mis)perception that virtual methods should be avoided because they were slower
that non-virtual.

-- chris

 

[Mike Schilling]

Now, I think that approach is overdone myself, but there is no
getting away from the fact that it is the only way to ensure
complete safety. If you don't follow it then a subclass can break a
superclass implementation without intending to, and (much worse) a
small change to an existing superclass can break existing
subclasses (called the "fragile base-class" problem). The message
from CheckStyle is telling you that you are not following that
approach.

Suppose I'm writing a base class for a hierarchy that processes bytes, and I
want to define the folowing signatures:

1. public void process(byte b)
2. public void process(byte[] barr)
3. public void process(byte[] barr, int offset, int len)

I'm happy to make 1 abstract, since it's the job of each subclass to define
what processing a byte does. I'm pretty happy to define 2 as

final public void process(byte[] barr)
{
process(barr, 0, barr.length);
}

since there should be no difference between an array segment and a full
array. But I really want to define 3 as

public void process(byte[] barr, int offset, int len)
{
for (int i = offset; i < offset + length; i++)
process(barr);
}

with javadoc explaining that this should be overridden when there's a better
way to process an array than byte by byte. The alternative is to make each
subclass that wouldn't override this method write this code itself.

 

[Chris Uppal]

Suppose I'm writing a base class for a hierarchy that processes bytes,
and I want to define the folowing signatures:

1. public void process(byte b)
2. public void process(byte[] barr)
3. public void process(byte[] barr, int offset, int len) [...]
with javadoc explaining that [3] should be overridden when there's a
better way to process an array than byte by byte. The alternative is to
make each subclass that wouldn't override this method write this code
itself.

Yes. This kind of issue is why I think that the
everything-should-be-final-or-trivial approach is over restrictive.

Personally, I am always tempted (and sometime yield to the temptation) to make
(1) and (3) provide mutually recursive default implementations:

=================
public void
process(byte b)
{
process(new byte[] { b });
}

public void
process(byte[] barr, int offset, int len)
{
for (int i = offset; i < offset+len; i++)
process(bar);
}
=================

People who don't read the documentation soon find out what their subclasses'
responsibilites are ;-)

-- chris