Suppose I have the following contrived code:
abstract class Root
{
public abstract void PrintHierarchy();
}
class Level1 : Root
{
override public void PrintHierarchy()
{
Console.WriteLine("Level1 is a child of Root");
}
}
class Level2 : Level1
{
override public void PrintHierarchy()
{
Console.WriteLine("Level2 is a child of Level1");
base.PrintHierarchy();
}
}
If I am only looking at the Level2
class, I can immediately see that Level2.PrintHierarchy
follows the open/closed principle because it does something on its own and it calls the base method that it is overriding.
However, if I only look at the Level1
class, it appears to be in violation of OCP because it does not call base.PrintHierarchy
-- in fact, in C#, the compiler forbids it with the error "Cannot call an abstract base member".
The only way to make Level1
appear to follow OCP is to change Root.PrintHierarchy
to an empty virtual method, but then I can no longer rely on the compiler to enforce deriving classes to implement PrintHierarchy
.
The real issue I'm having while maintaining code here is seeing dozens of override
methods that do not call base.Whatever()
. If base.Whatever
is abstract, then fine, but if not, then the Whatever
method might be a candidate to be pulled into an interface rather than a concrete override-able method -- or the class or method need to be refactored in some other fashion, but either way, it clearly indicates poor design.
Short of memorizing that Root.PrintHierarchy
is abstract or putting a comment inside Level1.PrintHierarchy
, do I have any other options to quickly identify whether a class formed like Level1
is violating OCP?
There's been a lot of good discussion in the comments, and some good answers too. I was having trouble figuring out exactly what to ask here. I think what is frustrating me is that, as @Jon Hanna points out, sometimes a virtual method simply indicates "You must implement me" whereas other times it means "you must extend me -- if you fail to call the base version, you break my design!" But C# doesn't offer any way to indicate which of those you mean, other than that abstract or interface clearly is a "must implement" situation. (Unless there's something in Code Contracts, which is a little out of scope here, I think).
But if a language did have a must-implement vs. must-extend decorator, it would probably create huge problems for unit-testing if it couldn't be disabled. Are there any languages like that? This sounds rather like design-by-contract, so I wouldn't be surprised if it were in Eiffel, for instance.
The end result is probably as @Jordão says, and it's completely contextual; but I'm going to leave the discussion open for a while before I accept any answers still.
Level1
violates (or seems to violate) the OCP. Even ifLevel2
didn't call base, I wouldn't think that it violates it either. OCP is a principle, not a pattern that says that you should call the base members on overridden members. – LebanLevel1
is violating the OCP just by looking atLevel1
's implementation: you don't know that itsbase
is abstract. (Hence "Ifbase.Whatever
is abstract, then fine.") – Bunchfoo(Level1 level)
) are distinctly different (the two things behave differently (print different things) whenPrintHierarchy
is invoked), hence how can we protect from the above mentionedfoo
doing its work relying on Level1 contract -- erroneously when supplied a Level2 instance? (the answer is that exact behavior ofPrintHierarchy
should be excluded from Level1's contract to foo to make it all work:) – Fredelbase
is called or not either. We might usebase
to help stick to it when appropriate, but if it's not appropriate for the methods purpose, doing so can be wrong. – Anaphylaxis