IronShay

Ironing code, geek t-shirts and even presentations!

NAVIGATION - SEARCH

My Leading Candidate for Worst C# Feature – Method Hiding

I love C#, I really do. Of course is has its little annoying quirks here and there, but in general it is, IMHO, one of the best static programming languages out there. Having said that, one thing that makes me wonder “WHAT THE HELL WERE THEY THINKNING?!?$?!?” every single time is the feature known as “Method Hiding”.

What is Method Hiding?

Method hiding, in short, is the crippled, mentally-ill brother of method overriding. For example, look at the next code:

class A
{
  public string GetName()
  {
    return "A"; 
  }
}

class B : A
{
  public new string GetName()
  {
    return "B";
  }
}

Class A has a GetName method; class B inherits from class A and implements the GetName method as well. 
Look carefully at the GetName method signature in class B – do you see the new keyword there? this means that the method doesn’t override the implementation in class A, it just hides it. What does that mean? read on.

So What’s the Big Deal? Hide, Override… Who Cares?

There is a huge difference in the behavior of method hiding and overriding. Look at the next two samples:

Method hiding vs. Method overriding

The left part is a method hiding example, and the right part is a method overriding example. Now let’s go through the use cases.

First – using the father classes:

FatherHidden fh = new FatherHidden();
fh.GetName(); // = "A"

FatherVirtual fv = new FatherVirtual();
fv.GetName(); // = "A"

Both are available and return the same result. Good.

Second – using the son classes:

SonHiding sh = new SonHiding();
sh.GetName(); // = "B"

SonOverriding so = new SonOverriding();
so.GetName(); // = "B"

Again, both methods return the expected result. Swell!

Third – using polymorphism – storing an instance of the son classes in a father class variable and calling the GetName method:

FatherHidden fh = new SonHiding();
fh.GetName(); // = "A"

FatherVirtual fv = new SonOverriding();
fv.GetName(); // = "B"

See that? the hidden method (FatherHidden.GetName) had suddenly woken up, took over and returned “A” instead of the expected “B”! kicking polymorphism out the door while doing it!

Is It a Problem?

Yes, it is. I’ve never found any reason to use method hiding and I can’t think of a good reason start using it in the future. OOP is great and I can’t understand why we need ways to screw it up. In my opinion, if you get to a situation where you need to use method hiding, re-think your design and start over.

This is not just a cute code smell. It can lead to nasty bugs along the way. For instance, I came across something like the next piece of code when doing a code-review lately:

class Base
{
  public bool IsAuthenticated { get { return false; } }
}
class SomeAuthClass : Base
{
  public new bool IsAuthenticated { get { return CheckAuthentication(); } }
}

Now, as long as they use SomeAuthClass variable types on their system, everything will work fine. But once a developer comes and wants to use some OOP magic, all users will immediately become unauthenticated. And this is no fun. No fun at all.

One of my major problems with method hiding is that it is C#’s default behavior – you don’t even need to write the new keyword. And even if the method on the base class is marked as virtual, and you forget to mark the method on the inheriting class with override – you fall back to method hiding…

#sadpanda

What I Am Suggesting

I know this feature isn’t going to disappear. Ever. I’m sure some people have found a reason to use it like there’s no tomorrow and Microsoft is one of the best in keeping their products backwards-compatible.

However, I would like:

  • To get a compilation error if a method is going to hide another method and is not marked with the new keyword.
  • To make the method hiding feature obsolete (yes, obsolete!) and get a compilation warning when using this feature in future versions of the framework.
  • You to stop using method hiding.
  • Everyone to recycle more and save the planet!

All the best,
Shay.

kick it on DotNetKicks.com Shout it



Comments (9) -

1. It already raises a *warning* (CS0108). If you want an error, turn on warnings as errors.
2. You can't remove it. You shouldn't *set out* to write such code, but it addresses specific scenarios where *you're not the author of all the code*.

Taking your first example, imagine A and B are in different assemblies, A provided by a third party, A does not currently have a GetName method, and B GetName is not marked "new".

You (and others) write code using your B implementation. If they want to invoke GetName, they obviously use a variable of type B, or further derived, since A doesn't have that method.

Now, the author of A comes along and adds a new method called GetName. Without method hiding existing, and being the default, now all of the derived code (B, etc) breaks, until such time as it's all recompiled. That's not good.

Of course, at the time you *do* recompile your B code, you now get a warning, which prompts you to investigate the new GetName function in your base class.

Reply

The authentication example should be rewritten to either:

- throw not implemented in the base class
- use an interface instead of a base class

Reply

United States Ron Warholic

Eric Lippert from the C# compiler team has addressed this before on his blog: blogs.msdn.com/.../method-hiding-apologia.aspx

In short--the purpose of method hiding is to make up for the lack of return type covariance on methods and give reasonable ways to implement patterns that don't use inheritance.

Reply

Yes, using method hiding where you have complete control over across the hierarchy, and you do not need to worry about breaking contracts is pointless - you shouldn't ever need it.

But that isn't the scenario it exists for.  c# is an OOP language, so it is valid to assume that some libraries will allow or require the library user to use inheritance, and method hiding helps overcome some of the brittle base class issues.  

Before you ask "what were they thinking", think through those other scenarios and figure out how you would solve them.  There are other ways, to be sure, but those ways create issues as well.  It's not nearly as clear cut as your simple single-case example seems to show.

Reply

I once wondered what method hiding was good for, so I asked stackoverflow.  As usual it was educational.

stackoverflow.com/.../is-method-hiding-ever-a-good-idea

Reply

United States Brennan Fee

This is a common misconception from younger developers who came in after OO became prevalent.  Generally there area always historic reasons for these "inventions".

The "primary" purpose for this is not simply to get around lack of covariance for return types (with all due respect to the brilliant Mr. Lippert).  Instead, it is to deal with a specific yet esoteric edge case that can occur in real world situations.

The primary purpose comes when you need a different method signature and don't want an overload.  Using your examples, perhaps you need a child class with a GetName method that receives a DateTime as input.  In this situation your only options are an overload - for languages that support them - or hiding the underlying method due to the method signature change (overriding would be an error and wouldn't compile).

While avoiding a divergence from the original contract should always be the goal, at times it does come up in real world situations (particularly when you don't own the base class as previously mentioned).  Whether you go with an overload or hiding the base method, you are presented with the same problem... polymorphism will "break".  Which is to say that casting the child class to its base will mean that the cast reference will have no knowledge of the new method.  (As your third example demonstrated.)

This is actually not a C# idiosyncrasy at all as many (if not most) OO languages support this sort of construct.  It stems from the nature of the way VMT's (Virtual Method Tables) are used to resolve a method.  By nature this constuct inverts the order of the method hierarchy and presents an alternate form of method resolution (bottom-up instead of top-down if you will).

Regardless, this technique should always be seen as suspect and scrutinized for other ways of solving the same problem.  In most situations a different design could be used to generate the same end result without the potential for "surprises".

That being said following a different design may be inordinately painful or impossible and so this technique presents the shortest best path to resolution.  Properly documented and moving forward with this option can get you past some genuinely sticky situations.

Reply

+1 for Damien.

This is a must feature to prevent future breaking of code when the base class changes.
The example is better understood when the method name is Init (who doesn't have an Init method..)

Reply

United States Nicholas Carey

While I agree with the auther that method hiding is (in general) a Bad Idea, the real problem lies in the implementation.

The problem is that method hiding is that, even though it comes with a warning, the default behaviour is to hide, rather than override. The default behaviour results in class hierarchies that violate Liskov's Substution Principal, that an instance of a subtype when upcast to its supertype should be completely substitutable for the supertype: the method using the supertype instance reference should not need to know the actual type of the instance. The default behaviour of method hiding result in the behaviour of the subclass changing when upcast and the user of the object needs to know what the exact type in order to know what to expect. If I invoke a method of an object instance, I should alway get exactly the same behaviour regardless of whether its been upcast or not.

The default behavour should be to do the expected — override — without having to specify the supertype's member as 'virtual' or the  or the subtype's member as 'override'. One should need to work to get to the oddball behaviour; one should not need to work to get the usual and expected behaviour.

Reply

Israel Itzhak Kasovitch

Aren't static analysis tools like FxCop and StyleCop provide rules for this situations. In not you can define a custom rule that produces a warning and then turn on TreatWarningsAsError so that the compiler will alert you when this happen.

regards,
Itzik

Reply

Pingbacks and trackbacks (9)+

Add comment