Use new keyword if hiding was intended

Question

I have the following snippet of code that's generating the "Use new keyword if hiding was intended" warning in VS2008:

public double Foo(double param)
{
   return base.Foo(param);
}

The Foo() function in the base class is protected and I want to expose it to a unit test by putting it in wrapper class solely for the purpose of unit testing. i.e. the wrapper class will not be used for anything else. So one question I have is: Is this accepted practice?

Back to the "new" warning. Why would I have to new the overriding function in this scenario?

Answer 1

The new just makes it absolutely clear that you know you are stomping over an existing method. Since the existing code was protected, it isn't as big a deal - you can safely add the new to stop it moaning.

The difference comes when you method does something different; any variable that references the derived class and calls Foo() would do something different (even with the same object) as one that references the base class and calls Foo():

SomeDerived obj = new SomeDerived();
obj.Foo(); // runs the new code
SomeBase objBase = obj; // still the same object
objBase.Foo(); // runs the old code

This could obviously have an impact on any existing code that knows about SomeDerived and calls Foo() - i.e. it is now running a completely different method.

Also, note that you could mark it protected internal, and use [InternalsVisibleTo] to provide access to your unit test (this is the most common use of [InternalsVisibleTo]; then your unit-tests can access it directly without the derived class.

Answer 2

You're changing the visibility without the name. Call your function TestFoo and it will work. Yes, IMHO it's acceptable to subclass for this reason.

Answer 3

The key is that you're not overriding the method. You're hiding it. If you were overriding it, you'd need the override keyword (at which point, unless it's virtual, the compiler would complain because you can't override a non-virtual method).

You use the new keyword to tell both the compiler and anyone reading the code, "It's okay, I know this is only hiding the base method and not overriding it - that's what I meant to do."

Frankly I think it's rarely a good idea to hide methods - I'd use a different method name, like Craig suggested - but that's a different discussion.