Abstract methods and the Open-Closed principle
Asked Answered
R

3

7

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.

Respondent answered 7/10, 2010 at 16:42 Comment(14)
Are you looking for a static analysis tool? If yes then perhaps this might help https://mcmap.net/q/935962/-are-there-any-static-analysis-tools-that-will-report-how-closely-the-solid-principles-are-followed/119477Banjo
@Conrad: I was hoping to achieve this using just my eyes and the compiler :) But if anyone else comes across this question and is looking for a static analysis tool, I would strongly recommend giving Nitriq a shot before diving into NDepend. It has a very capable free edition, and the full edition is quite affordable too.Respondent
I don't think that the method in Level1 violates (or seems to violate) the OCP. Even if Level2 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.Leban
@Jordão agreed, indeed as I said, you can violate it by calling the base member, if doing so changes the semantics of the method.Anaphylaxis
I think the point (and the problem) is that you can't tell whether or not Level1 is violating the OCP just by looking at Level1's implementation: you don't know that its base is abstract. (Hence "If base.Whatever is abstract, then fine.")Bunch
I think you can only tell that a class violates OCP if, when you need to extend its functionality, you can only do it by editing its source code.Leban
btw an(other) easy way to make one's head spin is to think whether Level2 is a subtype of Level1: their "full" contracts (as known at the place of their definition, not just inside some foo(Level1 level)) are distinctly different (the two things behave differently (print different things) when PrintHierarchy is invoked), hence how can we protect from the above mentioned foo doing its work relying on Level1 contract -- erroneously when supplied a Level2 instance? (the answer is that exact behavior of PrintHierarchy should be excluded from Level1's contract to foo to make it all work:)Fredel
I didn't quite follow that. But expected behaviors and contracts are related to another principle.... the Liskov Subtitution Principle (LSP).Leban
yeah, but both can be sure ways to confusion if one forgets about program correctness in the first place.Fredel
@Jeff, You can't tell that Level2 doesn't violate OCP either by looking at whether base is called or not either. We might use base to help stick to it when appropriate, but if it's not appropriate for the methods purpose, doing so can be wrong.Anaphylaxis
@Mark, I wonder if the focus on the OCP is distracting from what I take to be your core question, which I'd summarize as "do virtual methods (as opposed to abstract methods) have any role in good design?"Bunch
@Jeff: I'm not sure precisely what I wanted to ask here, but a comparison of virtual methods and abstract methods is probably on the right track. Also, I think we should start a club for SO members who have their dogs as their profile pictures... but I think it would only be the two of us so far.Respondent
If you're referring to Meyer's definition of OCP then it's likely this paradox isn't possible in Eiffel.Scorpio
Where is your client class in this example? When you add that, you'll see better the OCP.Scorpio
A
5

Root defines the following: Root objects have a PrintHierarchy method. It defines nothing more about the PrintHierarchy method than that.

Level1 has a PrintHierarchy method. It does not stop having a PrintHierarchy method so it has in no way violated the open/closed principle.

Now, more to the point: Rename your "PrintHierarchy" as "Foo". Does Level2 follow or violate the open/closed principle?

The answer is that we haven't a clue, because we don't know what the semantics of "Foo" are. Therefore we don't know whether base.Foo should be called after the rest of the method body, before the rest, in the middle of it, or not at all.

Should 1.ToString() return "System.ObjectSystem.ValueType1" or "1System.ValueTypeSystem.Object" to maintain this pretence at open/closed, or should it assign the call to base.ToString() to an unused variable before returning "1"?

Obviously none of these. It should return as meaningful a string as it can. Its base types return as meaningful a string as they can, and extending that does not benefit from a call to its base.

The open/closed principle means that when I call Foo() I expect some Fooing to happen, and I expect some Level1 appropriate Fooing when I call it on Level1 and some Level2 appropriate Fooing when I call it on Level2. Whether Level2 Fooing should involve some Level1 Fooing also happening depends on what Fooing is.

base is a tool that helps us in extending classes, not a requirement.

Anaphylaxis answered 7/10, 2010 at 17:11 Comment(0)
D
3

You cannot decide if a system (or class) follows the OCP just by looking at it statically, with no contextual information.

Only if you know what are the likely changes to a design, can you judge whether it follows the OCP for those specific kinds of changes.

There's no language construct that can help you with that.

A good heuristic would be to use some kind of metric, like Robert Martin's instability and abstractness (pdf), or metrics for lack of cohesion to better prepare your systems for change, and to have a better chance of following the OCP and all the other important principles of OOD.

Direct answered 8/10, 2010 at 14:7 Comment(0)
T
1

OCP is all about preconditions and postconditions: "when you may only replace its precondition by a weaker one, and its postcondition by a stronger one". I don't think that calling base in overridden methods violates it.

Tidewater answered 3/12, 2011 at 23:42 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.