Today I came to an interesting observation of public & private accessibility with regards to using the new keyword.
The scenario was exceedingly simple, almost too trivial to even write about but warrants a mention under the subtle but interesting observation as described below. First lets set the stage. The following program simply adds two items to a specialized list and then iterates over them to display them. The output is shown.
Now, lets assume you are asked to sort the output and are limited to only working with the specialized collection and the Node objects exclusively. So after some deliberation of what would be the optimal route from creating a SortableList (e.g. always sorted at the cost of a delayed insert) vs sort upon delivery, you go with the later and implement iComparable on the Node and decide to override the IEnumerable on the specialized list with the new keyword as shown.
The interesting observation here is the breakpoint inside the enumerator never hits. Why? This stumped me for some time and was in denial as Visual Studio wasn't reporting any errors or warnings. The keyword new is very powerful yet dangerous as it can be considered to bypass standard OOP principles and should be used sparingly because of these subtleties. Visual studio's lack of warnings, in this particular case, was of no help also.
The problem resides in the function signature not matching the signature expected by iteration -- particularly the public accessibility. As a result, you never actually hide the underlying one as expected. This is a very important observation and is the primary cause for the breakpoint not hitting. As a seasoned developer, I was expecting the compiler to warn on the use of new with a matching underlying function name but doesn't actually hide and replace it -- solely because of accessibility. To emphasize this further, you can rename the function anything else and and the warning will popup indicating the use of new is not required as nothing is being hidden so naturally if reverted and it doesn't warn when compiled, one can easily and wrongly assume it was hiding the underlying method assuming the existing public accessibility too.
Just out of curiosity of what's going on in the internals, I used reflection to inspect the methods with and without the public attribute with the following code:
MethodInfo[] enumeratorMethods = typeof(MyList).GetMethods().Where(t => t.Name == "GetEnumerator").ToArray();
foreach (MethodInfo mi in enumeratorMethods )
Console.WriteLine(String.Format("{0} - Public: {1} - Virtual: {2}", mi.Name, mi.IsPublic, mi.IsVirtual ));
And interestingly enough, with the public there are two methods and without public there is only one. In proper OOP, this would normally be impossible but yet is allowed in C# for subtle reasons.
In conclusion, by simply introducing public, the compiler is able to match the List<> class' implementation and effectively hide it causing the breakpoint to hit during iteration -- however we have a broken situation. In fact, if there was any other reference to iteration throughout the program against the MyList, they would all be sorted based on numeric value which clearly could be considered a break -- especially if the MyList was sorted, say alphabetically, for some other purpose prior to iteration elsewhere. Even a future user who decides to use the MyList API and sort locally by name for some output would stay scratching his/her brain as to why it resorts to sorting by number -- clearly not something you want to release to production. Many seasoned developers would never write code like this for this reason and would go through pains of finding an alternative more elegant and OOP compliant solution under the constraints given.
Moral of the story?...Be extremely careful when using the new keyword and only when you have to. Unlike override, where you can't change the accessibility and get nice warnings for anything violating basic OOP principals, with new you don't and could easily get in trouble -- especially down the road when further inheritance or some other matter is necessary as it will surely come back and bite you. |