views:

303

answers:

7

Hi all,

I'm writing a C# networking library (mostly as a learning exercise, it's not overly important to me if anyone actually ends up using it as I'm sure solutions are already out there).

I'm fairly happy with my structure so far... I have a few layers of client/server available, that can communicate in raw bytes over sockets, or slightly more complex through serialized message objects.

The question (problem?) I'm running into is when exactly I should declare a method, property, or event sealed, virtual, or with no qualifier.

I know what all of these do - sealed prevents inheritance of a class, or further overriding of a method. virtual will allow polymorphic behavior via method overriding.

Since I'm designing a class library, however, I'm not sure when to use these. It's a question of extensibility, I think... I provide some interfaces, an abstract class or two, and some concrete implementations for consumers of my library to use or extend, but I'm having difficulties deciding when it's a "good idea" to explicitly forbid derivation of a class or to allow overriding functionality.

Any general pointers or advice to keep in mind when designing my classes for use by others?

This question and this one were somewhat helpful, as was this one, but since I'm writing a distributable library I'm trying to cover all of my bases.

+1  A: 

Designing a class for extensibility via inheritance is, in general, not easy. When you seal something you're saying that the class really isn't suitible for implementation inheritance (maybe you're doing a bunch of low level stuff with unsafe code for instance).

For maximum composibility (in the mathematical sense), I suggest sealing everything that's not an abstract class. In fact, this is how get an Algebraic Data Type (in this case a Sum Type) in the language (check out http://blog.lab49.com/archives/3011 for a mind blowing article).

Rodrick Chapman
Very interesting article!
Sapph
+5  A: 

In my opinion, you can't really foresee what your users will eventually use this for. As such, it's generally a good idea NOT to seal anything unless there is some internal behavior you don't want users mucking about with (ie. they need to know that this has to be set first before you do that, etc..)

As for virtual, you would tend to make anything virtual that the end user may want to override. Events have reduced the need for virtual functions a great deal, but there are still times when you would want to make them virtual. In general, you need to think about how any given member function might need customization by an end user.

Mystere Man
+9  A: 

Start with the Microsoft Design Guidelines for Developing Class Libraries, particularly the Extensibility section, where you'll find articles on virtual members and sealing.

Quoting, here:

  • Do not make members virtual unless you have a good reason to do so and you are aware of all the costs related to designing, testing, and maintaining virtual members.
  • Do prefer protected accessibility over public accessibility for virtual members. Public members should provide extensibility (if required) by calling into a protected virtual member.

  • Do not seal classes without having a good reason to do so.

  • Do not declare protected or virtual members on sealed types.
  • Consider sealing members that you override.

Read the full articles, though.

Craig Stuntz
Leave it to MSDN. These articles are very helpful and I'll be certain to keep them bookmarked. Thanks! :)
Sapph
That MSDN article says not to use delegates and events in performance intensive areas, but how accurate is that still (considering the article is from 2005)? From my, admittedly limited testing, invoking a delegate is as fast as invoking an interface method, which is about twice as slow as a static method.
JulianR
Julian, this changed with .NET 2.0: http://www.sturmnet.org/blog/2005/09/01/ So you're right; that's out of date.
Craig Stuntz
There are certain philosophical objections to the 'not sealing unless you must' that I'm sure Jon Skeet will expounge at great depth :)
thecoop
+1  A: 

Controversial opinion: C# classes should be sealed by default.

If the class wasn't designed to be inherited and you haven't thought through the potential pitfalls or documented how to correctly inherit, then your class quite possibly will fail in strange ways if people override methods. When you don't seal your class, you are telling clients of your class that it is OK to derive from it and you will have to support this "interface" of your class in future to avoid breaking clients that choose to inherit from your class. This puts restrictions on how you can modify your class in the future.

So seal classes by default unless you explicitly want to allow it. And if you do want to allow it, make sure you document which methods should be overriden by making the virtual, and what the overridden methods must do to ensure that the class keeps working (such as calling the base method).

Mark Byers
I believe there's also a small performance benefit to sealed classes, as the runtime knows for sure that it doesn't need to look for overrides and such, and can optimize accordingly.
Joel Mueller
+7  A: 

First off, I agree with the other answers which suggest aggressively sealing anything that was not designed specifically to be extended. I do not agree that you need a reason to seal something; rather, you need a reason to leave something unsealed.

Your question is very general, so here's a general answer: your library presumably is a collection of features which can be used as tools to solve user problems. Each feature in your library is presumably there because you did some research, discovered that there was a user problem that needed solving, and solved it for them.

Each of those features has a certain cost associated with it. Some of that cost is in the past -- the time you've spent designing, implementing, testing, debugging and shipping the code. Some of that cost is yet to come: maintainance, reading bug reports, and so on. There are also more subtle costs, like perhaps maintaining backwards compatibility with one of your existing features will raise the cost of implementing a new feature tomorrow.

Extensibility is a feature too. It is a feature where if you get it wrong, the costs are almost entirely in the future. Treat it like any other feature: figure out whether it is a feature that your users really need, and whether its benefits pay for its costs. If you cannot clearly evaluate the benefit or the cost of extensibility then its pretty risky to implement it carelessly.

Eric Lippert
A helpful way to look at it, thank you!
Sapph
+4  A: 

There have been plenty of times I've said "Dammit! Why is this class sealed!?", but I've never said "gosh - I wish they'd sealed that class!"

There's a good chance that someday a better programmer than you will want to extend your class, and will know what they're doing. I think it's rare that there's a good reason to seal.

teedyay
+1  A: 

You have an interest conflict here.

  • If you wish to make it easy for your callers to test their code (e.g. to do test driven development), your users need to be able to mock your class.
    • so all methods ect must be virtual or you must have interface that you implement that contains all methods.
  • However if you wish to make it clear what contract you are providing on any subclasses your users may create, then you should keep to the Microsoft guidelines.
    • Do not make members virtual unless you have a good reason to do so and you are aware of all the costs related to designing, testing, and maintaining virtual members It is a pity there is not a way to mark a virtual method to say you don’t expect any client code to implement it apart from mocking.
Ian Ringrose