I saw someone (identity withheld) today on twitter besmirching Microsoft for making some classes in the .net framework internal. Some people (like Steve Harman) will say that you should make most everything public and virtual, and you know what, I pretty much agree with that (and I certainly respect Steve Harman as a developer, I love his blog). Unless you are dealing with nuclear reactors or pacemakers I again agree with Steve’s quote that you should be “giving sharp tools to sharp people” or as Guido van Rossum put it "we are all adults." In other words, don’t tie someone’s hands just because you think they might choke themselves with them. The problem is that there is a difference between tying someone hands and forcing them to think before they expose public methods on their classes.
On a publicly exposed class I see no problems at all with making almost all methods protected and virtual as long as you understand that someone somewhere is going to blow your class up in all sorts of spectacular ways. As I said in this post we all write crappy code and so it is likely that this person will one day be you. But again, I see no problem with giving developers powerful tools with which they can wreak havoc, but why would you want to litter your object with public methods that may not be needed? Why not allow people who want to inherit from your class to override your methods and even provide public methods to wrap these methods, without polluting the public interface of the class that they are inheriting from? Setting all methods public not only binds the framework developer into an interface and makes the objects harder to use because now we have to wade through a ton of public methods trying to find the ones that we want to call. And most likely you won’t want people to call a lot of methods at all, and I would argue that if you don’t have lots of methods in classes that you don’t think anyone would want to call then likely you haven’t broken up your code very well.
But so far all we have talked about is methods, so why don’t I think that all of the internal classes in the .net framework should be public? Well, because these sharp tools we are given allow us to wreak havoc and in most cases we want to be able wreak the same havoc across multiple versions of the framework. Let me reiterate again that I really don’t like the idea of tying the hands of the developer, but I am all for making the developer think before they do something. I shouldn’t keep the developer from doing something dangerous, but I should make them think before they do it.
One of these internal classes that I came across recently was ExpressionVisitor. This class is used extensively in the Linq namespace for walking expression trees and I needed similar functionality in my code for when I was building my Linq To SimpleDB app. At first I thought that it was a bit ridiculous that they didn’t expose this class for my use, but in reality it is an implementation detail. If they exposed this class for my use, then I would expect in the next version of the .net framework that this class would be there and would have relatively few (if any) breaking changes.
And therein lies the problem. In a public framework there are certain expectations that must be met for stability and maintainability. If Microsoft had exposed this class and then later on decided that the current implementation wasn’t sufficient and that they needed to fundamentally change the way in which the expression trees are traversed then they would have two choices.
1) Change the class and throw caution to the wind. This will cause anyone who has implemented this class to also change their implementation.
2) Leave the class in the framework and then add a new class that will be used in the future. I dub this class BetterExpressionVisitor.
So, what is the best choice? I guess that all depends on what is important to you. In my opinion, neither is a good choice. Choice number 1 leaves developers who used your class high and dry. How much does everyone complain when Microsoft drops support for a feature in an application? A lot. How much do people complain when they change the way the start menu looks? A lot. How much will people complain if they modify a class that thousands of developers are actively using? Well, I think you get the pattern here. Change number 1 will wreak havoc with lots of people and produce lots of whiney forum posts.
So, what about choice 2? Well, choice 2 is almost as bad in my opinion. Who wants a framework littered with obsolete code and cruft from past lives? If you want that, then by all means go use Java or Perl. (Oooooooooooh snap) But seriously, Microsoft completely invented a feature in .net 3.5 (extension methods) solely so that they would not have to heavily alter existing classes. Was *that* a good decision? I’ll leave that for you to decide, but in my opinion it was important for them to maintain compatibility between versions. And not just for this version, but if they added five thousand public methods on the ICollection interface then they would have to carry this through for version after version. I think that backward compatibility can be just as important, if not more so, in dynamic languages where people have a tendency to write code that relies on internal details of classes (see monkey patching).
So there you have it, I honestly think that most internal classes are not meant for public consumption should not be exposed just because someone somewhere *might* want to use them. If they want them that bad then they should just use reflector and steal borrow them. That way Microsoft can version their framework however they want and you won’t have to worry about anything breaking in the future because the class is now part of your codebase. Now, I know that this won’t work in all cases, but for crap’s sake people, can’t we just all get along? So, let me know what you think, should Microsoft make all framework classes public and just try to make as much backward compatible as possible or should they keep as much of their implementation internal so that developers won’t have dependencies on code that could change in the future?
Loved the article? Hated it? Didn’t even read it?
We’d love to hear from you.
Completly agree. I have a great example of that I ran intoa few months ago when working with the Web Part Framework in .NET. So, for the most part the web part framework allows you to do a decent amount of customization.
Some of the methods are bone headidly marked internal…so I need to use reflection to get to them. Perfect example: the method RenderVerbsInTitleBar. Who would ever want to override that and implement their own web part verbs in the title bar, right? 🙂
Permissive code is best in both cases.
In my opinion, you should think hard before marking anything private or internal – really the only things that deserve that kind of marking are things you’re trying to secure against malicious use, like critical security code.
Yes, someone might come along someday and misappropriate your code that’s not marked internal or private, but with Reflector, they might do it anyway. Ultimately the problem with internal and private are that they assume the author today is foreseeing every possible future use of this code tomorrow, and that as far as all of those future uses, everything they marked internal or private is perfect, and will never need a bug fix or enhancement for all time.
It’s fairly common for solid libraries to be written with a few bugs in private or internal code, then abandoned. Often this abandon comes in the form of a licensed product that’s dropped or moved away from, and the customer is now left with the tough choice of reimplementing what may be whole trees of internal classes, or finding some other cheaper hack, or writing the functionality from scratch – all to get the same result with one or 2 minor bugs fixed.
In short, internal and private assume what you’re marking with these is perfect, and will never need improvement. To assume you have that kind of foresight is obvious folly. So just don’t use them, unless it’s absolutely critical for rare reasons like security.