> > So, are you in the "no, not all methods should be virtual"
> > camp? If so, can't you see that having classes unsealed
> > by default is equally dangerous.
>
> No I'm not in the 'all methods should be virtual' camp. But
> I think the issues are rather different for methods. Firstly,
> there is the performance question and the fact that you
> cannot inline virtual methods.
Actually you can. Try saying that you can't on DevelopMentor's
ADVANCED-JAVA list (on the same LISTSERV as this), and see how long it is
before Jeff Kesselman jumps on you.
JVMs have been doing what's called monomorphic inlining of virtual methods
for some time now. This is a technique where the JVM detects that you are
not exploiting polymorphism, and behaves as though methods weren't virtual.
(It also adds checks to make sure that if you do start exploiting
polymorphism it will re-JIT the relevant code.)
Of course methods are virtual by default in Java, so there was more
incentive to develop technology that could inline virtual methods. But it
does illustrate that there aren't necessarily any perf advantages to
non-virtual functions.
> Secondly, if you make all methods virtual, then you
> lose the ability to make subtle decisions
> about how the class should be inherited. eg. if you have
> a Shape class designed for people to inherit from so
> that they can draw different single-color shapes, you'd probably
> choose to make the Draw() method virtual, but not the
> Color property.
But doesn't the exact same argument you've been using against sealed also
apply here? Why prevent people from doing what they might think they want
to do in inheritance scenarios? (I think it's a specious argument in this
case, but I think it is in the class case too.)
> Of course you'll say that in that example I've taken
> the trouble to design the class for overriding. If
> I've understood you correctly you want a class to be
> sealed unless you've specifically taken the trouble to
> design it for inheritance.
That's about the size of it. It's just a safe-by-default thing.
Inheritance is definitely in the category of things you should only do if
you really mean it.
Although as I said, this idea of making 'sealed' the default is not actually
what I think, it's just that I've found myself moving more towards that
opinion I think there are pros and cons to having classes sealed by default
and unsealed by default.
> I think my reply would be that even if you haven't spent
> any time thinking about inheritance, then at least other
> people should by default have the chance to inspect your class
> and decide, given its public and protected methods whether
> they can usefully inherit from it.
Again, surely you could apply that argument to virtual functions, where it
is manifestly wrong. If you haven't given any thought to how someone should
go about overriding a function, then the chances are they're not going to be
able to do it - they'll have no idea what the right semantics are: should
they call the base class, and if so before, during or after their own call,
and what invariants is the method supposed to uphold? And those are just
the straightforward ones - if you start getting into things like lock
semantics in a multithreaded environment it doesn't even bear thinking
about.
> You might not think they can do much with it but
> other developers will _always_
> come up with ideas you haven't thought of.
Which is precisely what troubles me. The general rule with code is that if
you don't test it, it probably doesn't work. So if someone derives from my
class to use it in a way I haven't thought of, I won't have constructed a
unit test for that scenario, so there's a considerably better than evens
chance of it not working.
Unless they're bolting on entirely orthogonal functionality. In which case
that sounds like a design error, and, more importantly, nothing they
couldn't have done without inheritance. Inheritance is not appropriate for
utility functions, or for composition of code.
> I think I'd find the arguments that you and Mike are
> putting for the dangers of inheritance a lot more
> convincing if you could come up with a concrete
> example of how you can break a class by deriving
> from it
That's a slight mischaracterisation of my argument - it's not breaking of
the class itself that really worries me. But this might be close enough to
keep you happy - it's an example of something else breaking because of a
class being derived from:
public class MyFoo {
private int spongVal;
public int Spong { get { return spongVal; } set { spongVal = value; } }
public override string ToString() {
return spongVal.ToString();
}
}
along comes a well-meaning developer who derives from this thus:
public class MySubvertedFoo {
private string nameVal;
public string Name { get { return nameVal; } set { nameVal = value; } }
public override string ToString() {
return string.Format("{0}, '{1}'", base.ToString(), nameVal);
}
}
So far so good - doesn't look like MySubvertedFoo has broken anything - just
added a new property, and has modified ToString. But what's this? Back in
the first assembly we find this:
public MyControl : Control { // Oo look! Inheritance, only good ;-)
[TypeConverter(MyFooConverter)]
public MyFoo Foo { get ... set ... }
...
}
public MyFooConverter : TypeConverter {
public override bool CanConvertFrom(ITypeDescriptorContext c, Type t) {
if (t == typeof(string)) return true;
return base.CanConvertFrom(c, t);
}
public override object ConvertFrom(ITypeDescriptorContext c,
CultureInfo ci, object o) {
if (o is string) {
string s = (string) o;
int spong = int.Parse(s);
MyFoo f = new MyFoo();
f.Spong = spong;
return f;
}
else
return base.ConvertFrom (c, ci, o);
}
}
So, what happens if somebody innocently does this:
myControl1.Foo = GetAMyFoo();
That will compile. And it's not particularly unreasonable. But what if
GetAMyFoo() actually creates a MySubvertedFoo?
Well it might work. But if someone manages to use it in the Visual Studio
.NET Forms Designer, (which would probably require the line of code above to
be in the constructor of a form from which someone else derives, in order to
get that line of code to execute in the designer) they would get a
FormatException if they tried to edit the Foo property of the control. (And
the same would happen if they were using the PropertyGrid outside of VS.NET.
Or any other piece of code which relies on the TypeConverter
infrastructure.)
So, we didn't break the class. We broke something else that was relying on
non-obvious semantics of the class.
OK, so it's a bit obscure. But then that's the point. And it's not
unrealistic. We hit problems because of unanticipated and totally
non-obvious semantic constraints. TypeConverters are ubiquitous and yet
most people don't know they exist, so it's pretty easy to make this
particular mistake.
How can you ever be confident that you're safe from all such presumptions
that other code makes about the class?
This is one of the biggest problems with inheritance. It is virtually
impossible to document fully the set of constraints on an object's
behaviour. (The exact same problem surfaces in scenarios other than
inheritance by the way. This is why it's so easy for a component upgrade
that ostensibly only contains bug fixes to break code that works just fine
with the older, supposedly buggier component.)
> As far as I can see, if you haven't thought about how a
> class would be used for inheritance then you
> won't have defined any protected members will you :)
> You'll have made all the members public or private.
> Derived classes won't be able to access private
> members. And you've presumably designed the public
> ones so they are safe to use. How can a derived class
> change that?
You've overlooked overrides of functions that were virtual in the base class
of the class you intend to derive from. Of course you could respond to that
by saying that override should default to sealed, and that you should
explicitly mark methods as virtual if you want derived classes to be able to
override them again... (I think that would be reasonable. I've never
really thought about this particular aspect before, but I don't think I like
C#'s defaults here. And it's arguably inconsistent - interface members use
virtual dispatch but are sealed by default...)
So supposing we run with a new policy - override *all* virtual methods in
the base class, and mark them as sealed. That would eliminate the problem I
just showed. So now the question is: what use is inheritance? What can you
do with it that you couldn't do without inheritance?
Can you show me an example of where it is useful to derive from a class in
the .NET framework that doesn't override any methods and doesn't use an
protected members?
For the record, I don't like the example you gave here:
> Even if you have a class with no virtual methods and
> nothing that looks useful for inheritance, other people
> might still wish to derive from it just for the syntactical
> convenience of having a class that implements whatever
> your class does plus some other features. One example
> of this is System.Drawing.Color. You can't derive from
> that because it's a valuetype - but if you could derive
> from it I would have done [1]. I'd love to have the
> syntactical convenience of having a color type with
> some extra methods like Darken(float fractionToDarkenBy)
> or some operators to add two colors together (add
> RGB components). **Such methods are completely
> orthogonal to existing methods**
(my emphasis)
So what are you doing using inheritance then? Such methods have no business
being in the Color class if they are completely orthogonal to everything it
does. (The ControlPaint class is the right place for, and where you will
find, the nearest equivalent to your Darken method.) The consensus in the
C++ community seems long since to be that such methods should actually be
global methods. (Global functions seem to have had a bit of a renaissance
in C++.)
I think this is classic example of what Craig was talking about when he said
that inheritance is like a mirage - from a distance it looks like what you
want, but turns out to be a disappointing pile of sand close up. You have
presented an attractive syntactical shortcut, which makes it look like
inheritance would be useful here. But with a little thought I think it
becomes clear that inheritance is The Wrong Thing here.
There's a very good reason why the two methods you describe should not be
members of a derived class (beyond the minor technical nit that they can't).
I could very reasonably want to use those on any Color. Why should I be
required to obtain your special ColorWithUtils just to use them? What if
someone else does the same thing with their own MagicColor class - I now
have two mutually incompatible versions of the Color class, and I have to
choose which one to use based on an arbitrary split across some utility
functions, the split being based on nothing more than who happened to decide
to write which utility functions.
Such methods clearly belong outside of the class. The usual heuristic that
the C++ community (or those C++ mailing lists I still subscribe to) uses is
that if a method is capable of doing its job by acting just on the public
members of a class, then it should be a global function. It absolutely
should not be a member of the class on which it acts.
Of course we don't have globals in C#, so we end up with statics instead,
which look an awful lot like globals in a namespace... So you end up with
methods like ControlPaint.Dark(myColor, myPercentage) which do exactly what
your darken method does, but can do it to *any* colour, not just your
special derivative.
Your argument is essentially one of syntactic convenience. I'll buy that
syntactic convenience is potentially a good thing, I just don't agree that
inheritance is the right way to go about it. It seems too high a price to
pay. And to be honest I've never found myself in a position where I've felt
even remotely tempted to use this trick. (So I don't agree that this
particular syntactic convenience would be all that useful. Exactly how much
typing is this really going to save you?)
> It's examples like that that you'd kill by routinely
> declaring classes as sealed
Good. :-)
> when there isn't a good specific reason to.
Sounds like reason enough to me... You've convinced me - I'm moving over to
sealed as my new default. Together we can stop this madness. ;-)
--
Ian Griffiths
DevelopMentor
You can read messages from the Advanced DOTNET archive, unsubscribe from Advanced
DOTNET, or
subscribe to other DevelopMentor lists at http://discuss.develop.com.
