Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Abstract methods and the Open-Closed principle

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.

like image 478
Mark Rushakoff Avatar asked Oct 07 '10 16:10

Mark Rushakoff


People also ask

What is meant by the open-closed principle?

In object-oriented programming, the open–closed principle (OCP) states "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification"; that is, such an entity can allow its behaviour to be extended without modifying its source code.

What is open close principle with example?

The Open-Close principle (OCP) is the O in the well known SOLID acronym. A module will be said to be open if it is still available for extension. For example, it should be possible to add fields to the data structures it contains, or new elements to the set of functions it performs.

What is open-closed principle in agile?

The Open-Closed principle states that code should be "Open for extension" and "Closed for modification". There is quite a lot of confusion about the term, but essentially it means that if you want to implement a supported change, you should be able to do it without changing the code in a large number of places.

What is the limitations of open-closed principle?

This has several disadvantages: The resource allocator code needs to be unit tested whenever a new resource type is added. Adding a new resource type introduces considerable risk in the design as almost all aspects of resource allocation have to be modified.


1 Answers

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.

like image 162
Jon Hanna Avatar answered Sep 20 '22 20:09

Jon Hanna