Saturday, August 15, 2009

Do Not Make Every Method Virtual

I’m reacting to Roy Osherove’s recommendation, “make methods virtual by default”, in his excellent The Art of Unit Testing which I reviewed a few days ago.

I’ve heard my friend Jeremy Miller wish that .NET methods were virtual by default as they are in Java. I respect these guys immensely but I strenuously disagree. You should make each member virtual reluctantly and in the full knowledge of the risk you are taking. I hope you’ll at least appreciate my concerns even if you are not convinced.

Why Virtualize All Members By Default

It’s Roy’s first suggestion in his “Design for Testability” appendix [258]. He calls it “handy” and it is handy indeed. The easiest way to stub or hand-mock a class is the “extract and override” approach in which you override a production class to gain access to its innards during testing.

In one such example [71], a TestableProductionClass derives from a ProductionClass so that the implementation of the latter’s GetConcreteDependency() can be replaced with an alternative that returns a stub class instead of the concrete class.

// ProductionClass
protected virtual ILogger GetLogger() { return new ProductionLogger();}


// TestableProductionClass : ProductionClass
protected override GetLogger() { return new TestLogger(); }

What an easy way to swap out the dependency on the production logger … and I don’t need any of that messy IoC machinery.

If every method were virtual, any member could be replaced in this fashion within the test environment.

Making every method virtual makes life easier for the mocking frameworks too. Most do well at fabricating mocks either for interfaces or for unsealed classes with protected virtual methods. I believe it is fair to say that most have a tougher time mocking closed concrete classes.

A class with all virtual methods is easier to proxy too, lending itself to injection techniques such as Ayende demonstrates in his post about injecting INotifyPropertyChanged into POCO objects.

What could possibly be wrong with this?

Invites Violation of Liskov Substitution Principle

The “Liskov Substitution Principle (LSP)” is the “L” in Robert “Uncle Bob” Martin’s SOLID design principles. Formally this principle states that the derived class can’t make a method’s pre-conditions stronger nor post-conditions weaker. In colloquial English, the derived method can’t change the fundamental guarantees of the base method. It may do things differently but it shouldn’t violate our expectations of what goes in, what comes out, or what the method does.

Uncle Bob, in his Agile Principles, Patterns, and Practices in C# , describes LSP as a “prime enabler of the Open Closed Principle (OCP)” [151]. It follows that a violation of LSP is effectively a violation of the more familiar OCP.

Now I have no problme with deliberately making some methods virtual. That is one of the favored techniques for facilitating OCP; it is a mechanism for identifying an opening for extension.

My argument is with opening up the class blindly and totally by making everything virtual. Suddenly nothing in the class is truly “closed for modification.” The “virtual” keyword announces to the world “here is my extension point.” When every method is virtual, the world is invited to change every method.

The lesson of Martin’s LSP chapter is that extending a class by overriding a virtual method is tricky business. The innocent seeming Rectangle is his famous example. What could go wrong in deriving Square from Rectangle and overriding its Length and Width properties? It turns out that plenty can go wrong [140] and that it’s almost impossible for the author of the class to anticipate and guard against the abuse of his class.

Martin is pragmatic. “Accepting compromise instead of pursuing perfection is an engineering trade-off. … However, conformance to LSP should not be surrendered lightly.” [his emphasis, 149] . He continues:

The guarantee that a subclass will always work where its base classes are used is a powerful way to manage complexity. Once it is forsaken, we must consider each subclass individually.

I’m going to come back to this point. Because we give up the guarantee … and open wide the door to big trouble … when we make every member virtual.

Let back up a second and elaborate on the danger.

The Wayward Elevator

I am the maker of Elevator software that runs elevators around the globe. My Elevator class has an Up method. Suppose I make it virtual. How might value-added elevator developers implement an override of a virtual Up in their derived BetterElevator class? They could

  • replace it completely; base.Up() not called
  • call base.Up(), then invoke custom logic
  • invoke custom logic, then call base.Up()
  • wrap base.Up() in pre- and post-logic

It’s not an issue when I am doing the deriving. I almost never call base when I derive a test class (TestElevator). If I support construction of a dynamic proxy to address cross-cutting concerns, I expect the proxy to wrap the base method. These scenarios are not worrisome to me. Why?

When writing tests, I have intimate knowledge of Elevator. Elevator’s details are open to me. I wrote it. I know what I’m doing.

If I make Up() accessible to a proxy, I again know what I’m doing … and more importantly, I know how the proxy will manipulate my method. The proxy may behave blindly but it operates automatically and predictably. Whether I decorate the method with attributes or map it or rely on some conventions, I, the author of the class, am choosing precisely how it will be extended.

Unfortunately, I can’t declare Up() to be “virtual for testing” or “virtual for proxying”. It is simply “virtual”, an invitation to extension by unknown developers in unknown ways. I have lost control of my class.

I knew that Up() sent the elevator ascending. But I can’t stop someone from re-implementing Up() so that it descends instead. Maybe base.Up() triggers the doors to close. The developer might call base.Up() too late, sending the elevator in motion before the doors have closed. The developer could replace my base.Up with something that juggled the sequence of door closing and elevator motion methods, interleaving custom behaviors, yielding upward motion that failed to satisfy some other elevator guarantees.

Any of these or a hundred other implementations of Up() could alter Up-ness in ways that are subtly incorrect or catastrophically wrong. Everyone one of those implementations requires that the developer understand intimate details of the Elevator class, details that he would not have to know if the method were sealed.

“Up” is definitely not closed to modification. It is dangerously open. As an Elevator man, I should work hard … perhaps writing a lot of Elevator.Up certification tests … to ensure that essential pre-conditions, post-conditions, and possible side-effects are all correct even for derived classes.

The burden on my development has gone up as well, not through conscious design but by default. This is unthinking design, fiat design. My code fails “open” instead of “failing” closed. I better be very good and very conscientious about manually sealing what the language would make virtual on its own.

I’m not that good and I’m definitely not conscientious.

Is This Paternalism?

Am I being absurdly protective. Sure there are risks. Programmers are adults and we should treat them as such.

I hear this a lot. I hear about how Microsoft tends to infantilize the developer, tries to shield them from the bumps and bruises that are essential to mastering the profession. There comes a time when you let the kid have sharp scissors and run around with them if he must. A few blind kids is a price worth paying.

I agree … as long as I’m not paying the medical bills.

I Write Frameworks, You Write Applications

I can’t tell you how to build your applications. When you own the class … and you do as an application developer … your team is answerable to you. You don’t come to me with your medical bills.

But I write frameworks for you to use. You’ve licensed my product and you’re paying me for support. When the elevator goes down instead of up; when the doors close suddenly and injure a rider; when the elevator simply stops … you don’t say “I wonder what I did?” You say “that elevator we bought is a piece of crap.”

You paid for support. You call me. And I am pleased to give it.

It’s not free to you. It’s not free to me either. I have to find what’s wrong as quickly as possible and get your elevator moving safely again. Unfortunately, if you’ve written BetterElevator (as you are supposed to do … it’s a framework remember) and you can change everything about my Elevator, I face an extremely challenging support call.

I have no idea what you’ve done. I don’t know what you’ve done to elevator and I can’t guess. You can tell me … you will tell me … that you haven’t touched “Up”. Perhaps you didn’t. Instead you’ve overridden another method on another of my classes that Up requires.

Maybe you don’t write applications. Maybe you write an open source framework. Fantastic. You don’t have true customers. If someone has a problem, you diagnose it and maybe you fix it … at your leisure. That someone has no recourse … and knows that going in.

That’s often why businesses won’t use open source. As more than one manager has told me, “I want a neck to wring.”

I think I’m entitled to fear for my throat. But that’s not my real motivation. I’m in the business of providing good service for a product that makes certain behavioral guarantees. I can’t deliver good service if I can’t make those guarantees and I can’t make those guarantees if every method of every class is up for grabs.

I’m not sure you can either.

.NET Framework Design Guidelines

I’m not the only guy with a healthy suspicion of virtual methods. I admit my degree of suspicion is higher than the that of the application architect’s. The application architect probably knows and controls the developer who derives from his class. The unknown developer who derives from my class controls me.

The designers of .NET understand this too well. That’s why in the .NET Framework Design Guidelines, Krzysztof Cwalina and Brad Abrams write

Virtual members … are costly to design, test, and maintain because any call to a virtual member can be overridden in unpredictable ways and can execute arbitrary code. .. [M]uch more effort is usually required to clearly define the contract of virtual members, so the cost of designing and documenting them is higher.  [201]

If all members are virtual by default,

  • I waste effort manually sealing most of them,
  • I test and document and support more methods than I have resources to support
  • I add unwanted complexity

and what do I gain for my pains?

What Should We Do?

I want to blame someone. I’m going to blame the language authors. Maybe methods are automatically virtual only in test environments. We should be able to mark methods for proxying and compile time injection (as in static Aspect Oriented Programming). Otherwise, members are closed unless explicitly made virtual through the conscious effort of a conscientious programmer.

Meanwhile, I’m prepared to be pragmatic. I like Roy’s recommendation [260] that dependencies should be defined in virtual properties (or methods) without logic.  Auto-properties and similar logic-less methods are safer to make virtual. The Template Design Pattern is a controlled approach to extensibility that may assist testability as well. Interface-based designs help too.

That’s as far as I dare go as a framework developer. Application developers may have more rope to hang … er … more latitude.

25 comments:

Anonymous said...

Can't say i'm completely convinced.


What the "elevator" example demostrates is that we should not make all methods virtual, since it exposes extension points where we do not want them. Lets have a look at the reason this particular extension point is not wanted. The elavator.Up() method has responsibilities for moving up and closing doors etc, and these need to be chained in the right order to function safely; we do not which a client to break this by inadvertantly calling up() at the wrong time. For me, this just highlights the smell of breach of SRP in the elevator class. Since this issue is cause by incorrect ordering of responsibilities, by adhering to SRP we would avoid this conflict. We can then keep this method as virtual, and benefit from an additional extension hook safely.


Paraprahsing another point made (correct me if this is a mis-representation) is that we may want to be very explicit about the points in which and application can be extended. Marking a method as virtual is a way of explicitly showing your "approved" extension hook.


Arguably, I prefer to see my "extension hooks" more explicitly that a virtual method. As an application developer looking for a "seam" to extend, I would favour seeing an interface that I can implement, over inherting from a class and overriding it's virtual methods. This is mainly due to being that interfaces tend to be more discoverable than virtual methods.



Lastly, I have a different perspective on this OCP. I believe "closed for modification" to be in reference to the actual LOC contained in the class rather than "closing off" modifications to behaviour. I agree however, that making all methods virtual does open up a greater susceptibility to the violation of LSP.



The arguements you have made mostly seem to fit in the umbrella of protecting yourself against unwitting developers, and I think there are safer ways to do this while still obtaining the flexibility provided by making all methods virtual by default.

Mark Nijhof said...

I replied in a blog post myself: http://blog.fohjin.com/blog/2009/8/16/Running_with_scissors I agree much with the previous commenter :)

Nick said...

I agree wholeheartedly with you, Ward.

It is a matter of taste, perhaps, but I'm not willing to sacrifice code quality (carefully-considered, reliable extension points that maintain invariants) for testing convenience.

Then again, I use interfaces frequently and thus sidestep many issues associated with mocking concrete classes.

Ward Bell said...

@Mark - I left the following coment on your blog.
---
Mark - Thanks for giving my post a serious reading.

I addressed your point directly in the section "Is This Paternalism?" where I tried to establish that protecting developers from themselves was _not_ my motivation. I think that's noble, but I wouldn't do so at the cost of inhibiting the expert developer.

I limit the surface area of my classes in order to deliver a high quality, thoroughly designed product that you can count on. Remember I am a framework provider. You are building on my classes. You trusted me to do a job. If you trust me, let me do it.

I'm making the case for encapsulation. If I have closed off some of my code, it is because that code is pure implementation. Reflection may reveal my implementation, but by announcing via limited accessibility that certain classes and methods are off-limits to you, I'm doing my best to draw a bright line between extensions points I support and implementation that is mine to change at will.

I could try to annotate my classes with comments to indicate the boundaries as you suggest. In fact, I have had to do so in those cases where peculiarities of .NET oblige me to make something public that should not be. I'll say "Internal IdeaBlade Use Only".

These comments have not been effective in practice. They are routinely ignored, largely because people don't read comments. They get angry when the signatures change, the classes disappear, or their side-effects are

Do you make your fields public? Of course not. Why does your reasoning not apply to fields? If I may answer for you in one word, it would be "encapsulation". Encapsulation frees you to make changes without altering consuming code.

YAGNI reminds us that we should not over-design. Many of these non-public classes and non-virtual methods are just good enough to satisfy as implementation. They often lack guard logic, for example, because I can be sure when and how they are called. They may rely on facilities that I _know_ will change, be superceded, or even disappear in a forthcoming release. Sometimes they are just half-assed. We can't perfect everything; we choose what to polish and what to leave rough.

I cannot ... and I should not ... expose these implementations to the wild. I don't have unlimited time or resources; no one does. It would be crazy to sacrifice valuable features just so I can supply you with documentation and certification tests for functionality I never wanted to support in the first place.

Let me return briefly to the original point of my post. I don't want my argument against virtual-by-default to hinge on the occasional necessity of sub-par implementation. When I seal a method it is because I want to make a guarantee that the method will do what I said it will do. Making it virtual can void that guarantee, obliging me to prop it up with costly, perhaps fruitless efforts, that I could easily avoid. I may choose to make it virtual anyway. That's a design decision, not something foisted upon me by an edict from on high.

It's important to get back to why you want virtual (or public) in the first place. Your desire to simplify stubbing, mocking, or proxying of my classes is absolutely on the money. The CLR should help you realize that desire. But it doesn't. Your wish, however worthy, is not a good enough reason to put at risk my product quality or my product roadmap.

I'm not trying to protect you from yourself. Call me selfish but I'm trying to protect the future of the product and give you the service you paid for.

Ward Bell said...

@craigcav - I was kind of hoping someone would attack the Elevator.Up argument from the point of SRP. Call it a set-up.

First, I never said that the method itself knew how to do all the things necessary to make the elevator move properly.

Second, something has to be responsible for coordinating elevator movement. You'll just be chasing your tail if you keep factoring it away. I invite you to pseudo-code your way out of this one.

Third, there is no pixie dust that reveals and corrects SRP violations. If it were that easy, we wouldn't be having those WTF moments everytime we re-read our own code. Not all methods can receive or deserve the attention that a true extensibility point commands. Which is a reason to be cautious about making members virtual.

I agree with you, "marking a method as virtual is a way of explicitly showing [an] extension hook." That's why I favor virtual-by-choice over virtual-by-default.

I share your preference for interface-based extension points.

Ward Bell said...

@Mark - I want to add another to my inventory of "humble programmer" remarks.

Getting method names and signatures "right" is very difficult. It's especially difficult when you have very few use cases.

In many cases, I suspect that a method I've written will be generally useful. But I have no evidence for that nor evidence for the resonance of my method name nor evidence that the signature that suits my needs will be a good one in the field. I only know it works for me ... today.

Experience teaches that I should not commit without customer evidence. Maybe I just have a problem with commitment.

Caution and patience are often rewarded. We release an updated version of DevForce every six weeks like clockwork. When the customer makes a compelling case for another extension point or access to hidden functionality, we are able to respond quickly.

Mark Nijhof said...

Hi Ward,

Thanks you for the elaborate answer!

I read the 'Paternalism' part as; its ok if people run with scissors and fall, as long as these where not my scissors, perhaps a misinterpretation from my side :)

I may have mixed two concerns in my post by having the virtual by default and internal and private discussions together. They are closely related but different in how you can change the functionality.

Using virtual you can change the functionality of the class implementation itself. And yes this is to facilitate mocking and proxying the classes. And I can see your point there that you want to ensure that the behavior that you specify is always going to be the behavior of that class. But if the language would have supported the virtual by default and have a keyword to indicate that things should not be virtual, then especially in testing usage of a third party framework things would go a bit smoother. But you could still specify places where you want non virtual behavior.

As for marking entire classes as internal or private, I think this is a shame because it limits you from ever making a different implementation of one of these. And especially for a framework this could be important, as when I create an application I usually have enough freedom to make the changes I need, but when using a framework I have to use what is provided, and frameworks don't always behave in such a way as I need them to.

Now I am no framework builder, I only contribute a little bit to FubuMVC, but there the whole framework (granted that it is relative small) is open and nearly all behavior can be replaced by something custom. I think this offers great value, but yes imposes greater risk.

One thing I am looking forward to is the Design by Contract implementation in .Net so you can specify DBC on an interface and ensure it acts the way you need it to be but still providing complete freedom in the implementation. Really a shame that Spec# was not completely incorporated since there it was part of the language which makes for nicer syntax. I can see this having great benefits for framework designers.

Great posts I learn a lot from reading them!

Mark Nijhof said...

Hi Ward,

No need to be so humble :)

I confront ideas that are different from my own so I can learn and change my understanding of them.

I can absolutely agree with the fact that developers don't listen or read comments, we all know best right :) it is a real shame.

I also think there is a difference between one big black box and many smaller black boxes that can actually be changed by different functionality.

But hiding it is still difficult to justify for me, perhaps I first need to support a large framework to fully understand the value of it?

Ward Bell said...

@Mark - Yes, I was a bit uncomfortable extending my argument from virtual-by-choice to cover non-public classes. There are similar issues but enought differences to warrant separation.

I lean toward sealed-by-default because I believe that extensibility requires thought.

We could auto-generate tests too. Why do we oppose that? Because we believe you should think about your code ... think through your expectations. Is this paternalism? Nothing stops me from adding expectations. Why not benefit from automated coverage?

I put a different face on it. Good programmers write expectations. We want to encourage good programming. We want to discourage bad programming. Therefore we act paternally.

I feel the same way about virtual. Good programmers open up by design, not by accident. Good programmers think about the implications. And when they think, more of their logic-rich methods are closed to modification than open to extension. .NET's default reflects that discovery. So we default both in support of good programming and to reduce risks of bad programming.

I share your desire for CLR changes that will help us have our cake and eat it too.

Commercial frameworks are different from open source frameworks in how they manage risk. The open source consumer has a higher risk profile, is willing to investigate internals and change the framework if necessary, and is willing to endure longer periods between updates.

The consumer of commercial frameworks is paying for a different experience. She doesn't want to read the source or modify it - that's my job. If something is wrong she wants me to find and fix it ... now. She values a commercially supported platform with guarantees and a committed roadmap over a community supported platform with unlimited options.

As is often the case, how you come down on "design principles" is conditioned by the nature of the application and its consumers.

Thanks for all your careful thoughts.

Anonymous said...

Haha, and I took the bait, hook line and sinker! You're quite right, you never did say that the method itself knew how to do all the things necessary to make the elevator move properly.

I can definately see a need to be certain that certain code cannot be overridden. For example, if the logic co-ordinating the other parts (doors closing) lived in the Up() method, or rather, the Up() method invoked the execution of some pre-co-ordinated behaviour, we'd still need to be assured that the execution of the behaviour was guarenteed. If this was the sole responsibility of the co-ordinator though, why not make this class sealed? We can still manage our invarients carefully, whilst providing only carefully considered extension points.

I will admit, I'm playing devils advocate a little here, since my feelings about this aren't that strong either way.

Mark Nijhof said...

Ward,

I do value the need to think about the code that you write, that is also why I see great value in TDD and Pair Programming, both basically trigger you to think more about the code you produce. So I can definitely see your point on that with respect to extendability, but instead of marking something as virtual, a actual extension point makes more sense to me, but as I said that could be because of different (less) experience.

I do see the difference between commercial and OSS frameworks with respect to consumer expectations, could that difference explain why OSS is 'perhaps' more innovating? (ps not implying anything on your framework).

Thanks again for your thoughts on this!

-Mark

Mark Nijhof said...
This comment has been removed by the author.
Anonymous said...

Ward, thanks for making me think about this topic again. I'm definitely in the camp of wanting all things virtual by default, though. The truth is that I've been hurt more by not being able to override something than by overriding something incorrectly. In fact, I can't remember ever having this problem. I certainly understand your perspective, though; particularly as it relates to supporting a product. I think that the "virtual" by default issue is less of a problem as you represent it if we favor composition over inheritance. As craigcav has already stated, interfaces help create very explicit seams within an app/framework. Unfortunately, most of the .NET framework favors inheritance over composition and this often forces framework/app developers down the same path. Thinking compositionally is difficult, but rewarding. It goes hand in hand with OCP.

Ward Bell said...

@Rob - always great to see you weigh in.

I agree with favoring composition over inheritance.

Not to be too clever, wouldn't you think that a preference for composition speaks against virtual-by-default? Why would we invite by default the design choice we prefer least?

In any case, that isn't an option in this debate. The question on the table is "Should methods by virtual by default?"

I too have been frustrated by closed elements of the MS platform. I've been frustrated by closed elements of my own product. I'd like to see some interfaces where they are missing and some injection points that aren't there.

I've battled for them. And when I lose, it is usually because I have been persuaded that opening that particular door is opening a Pandora's box. I'm obliged to rethink how I can offer the extension point without risking the integrity of the product.

I don't know how many years Caliburn has been in production. Our product is in its eighth year and we have kicked ourselves more than a few times by exposing something that shouldn't have been exposed or wasn't ready or no one wanted (we have to test it as an extension point anyway) or was just a bad idea poorly executed.

We also have the luxury of continuous development. This is our job, not our avocation. We release every 6 weeks. We can open a seam in the product quickly in response to actual, not just anticipated, customer need. If there is a good reason to expose a class or make a method virtual, we can do so in a relative snap.

@Mark - I don't think there is reason or evidence to believe that OSS is better or more innovative than commercial software. I have a feeling we could fill a score card with innumerable examples of good and bad from both sides.

My suspicion is that factors such as talent, resources, competitive pressure, and consumer focus have greater explanatory value. Such factors surge and ebb over a product's lifetime.

Thank you, all.

Anonymous said...

Hi Ward,

That is an interesting point you bring up concerning the relationship between OCP and LSP with reguards to virtual by default.

I have always interpreted OCP to mean, "as long as I do not have to modify the base class to accomplish my goal."

Thanks for the new perspective. I will have to mull it over the rest of the day.

Peter Leslie Morris said...

If you write code that does the opposite of what you want, you don't get what you want.

I can descend from a non-sealed class, override the constructor, and throw a NotImplementedException - so what?

The ONLY argument I can think of against making methods virtual by default is that it *might* run a bit slower, but I doubt it would be an important factor in any app.

Julian birch said...

I'm afraid I think you're misinterpreting the OCP. The OCP says that you shouldn't have to change the code in a class except to bugfix. And by change the code, we mean actually edit the code in the class we're talking about, not overriding it in a child class. The more methods you make virtual, the /less/ likely you will have to violate the OCP.

Liskov is important, but observing Liskov is the responsibility of the inheriting class. Just because you've enabled people to do the wrong thing doesn't mean your design is faulty.

Mats Helander said...

I don't agree with the premise that you are using here, that using non-virtual methods (in a non-sealed class) would somehow be "safer". I posted my opinion in reply to Ayende's post, but since his post was in reply to yours, I'm putting the link to my reply in your comments to.

http://matshelander.blogspot.com/2009/08/are-non-virtual-methods-safer.html

All the best,

/Mats

Ward Bell said...

@Peter Morris - I am sympathetic to the premise that the developer bears responsibility for what she does with a class. It is her fault if she breaks it or misuses it. I don't disagree one bit.

I'm making a different point. My stance is that I, the author of a class, take responsibility for how my class can be extended. When I make a method virtual I am announcing "here is an extension point that I am consciously making available to you. I have thought through how you might use this and written it as best I can to support you when you override it."

When every method is virtual, the code itself no longer makes clear where are the intentional degrees of freedom. I could resort (as some have suggested) to comments that properly differentiate true extension points from "virtuals" that exist solely to facilitate testing or proxies. That has not worked so well for me. Most modern designers (see Uncle Bob) do not look favorably on code comments anyway.

I'm not staking out a bizarre position. I'm completely in line with SOLID principles. Look at the purpose of the Template Method Design Pattern:

"The template method is used to let subclasses implement (through method overriding) behaviour that can vary ... and control at what point(s) subclassing is allowed."

We would only need this pattern because, as class designers, we want control over how our class is used.

As a commercial framework designer I need that control. I have to balance your freedom with your requirement that I provide the support and consistency you paid for.

Not everyone will strike the same balance. So much depends your relationship to developers using your product.

Ward Bell said...

@Julian Birch - If I am misunderstanding OCP I am in great company when I do so.

As I said in my post, Uncle Bob Martin, in his Agile Principles, Patterns, and Practices in C#, describes LSP as a "prime enabler of the Open Closed Principle (OCP)" [151].

The SOLID principles are mutually reinforcing. They are all distinct but they all have a way of coming back to how we achieve the benefits of SRP and OCP.

I do not claim that Uncle Bob endorses my position on virtual-by-default; I have no idea how he feels about it. I observe with satisfaction that, in all of his approved OCP code samples, there are a mix of virtual and non-virtual members. Every virtual method is virtual-by-choice.

He has numerous opportunities to declare himself in favor of virtual-by-default. Why not just make it easy and be infinitely closed to modification by making everything virtual? Simple rules are easy to follow.

He doesn't even hint at this possibility. I'm confident he would entertain seriously my perspective and my line of reasoning.

Yes, OCP, narrowly understood, prohibits modification of the class source code for purposes of extension.This is the responsibility of the class author.

In contrast, Liskov is the responsibility of the inheriting class .. as you rightly point out. I remain convinced that the author of a class shares some of that responsibility ... by making the class as safe and easy to extend as possible. The challenge with virtual methods, as I noted, is that the developer who would derive from the class cannot tell whether to call the base method and, if so, in what order. She must break encapsulation to find out.

One great thing about the Template Method Design Pattern is that it is abundantly clear how you should override it: you replace it completely; you don't call base (which, being null, is harmless if you do).

Another is that the method name (it should have the suffix "core") announces its purpose by convention.

Ward Bell said...

@Mats Helander - Thanks for the link to your post. You really put your back into making your case. I appreciate that you think the subject itself merits consideration.

Thanks for the link to Ayende's response to my post. Ayende's thoughts have inspired much useful commentary pro and con.

And thanks also for the reference to Anders Hejlsberg's explanation of why he decided against virtual-by-default. He made my case succinctly.

Your observation about "binding to an interface" resulting in low coupling is right on. I hasten to add that it also provides almost zero behavioral guarantees. It is vacuous by nature. I remain amused that people think of an interface as a "contract". In what respect does an interface resemble a contract in the real world? Completely escapes me.

I'm having trouble following why we're talking about interfaces at all. Eventually one must write a concrete class that actually does something. People don't buy my framework for its interfaces. They buy it for its concrete implementations. And the question on the table is "do we make its classes' methods virtual-by-default ... or not?"

I'm probably too punchy to understand why you seem to argue so strenuously against "binding" to one of my concrete classes ... or a derivative thereof. Are you against inheritance altogether?

No matter. I'm now at the point where I feel the differences of opinion have been thoroughly aired and I've about run out of things to say. I hope you agree that there is intelligence on both sides.

I wish to add only one more point. Mine is a living framework. We release a new version every 6 weeks. If we fail to provide an extension point that you need AND it makes sense, the cost to you is at most a six week PITA. That hurts ... but let's not get apoplectic about it.

I concede that by closing methods I'll never know the marvelous things that people might have done had they been virtual. That's a risk I'll take.

Mats Helander said...

Ward,

Thanks for reading my post,

"I hasten to add that it also provides almost zero behavioral guarantees"

Binding to an interface provides _exactly_ zero behavioral guarantees, and that's the whole point! :-)

As a default perspective, this should be thought of as a *good* thing! If you _can_ get away, in your given scenario, without having to demand behavioral guarantees from your services, so much the better, right?

But, you might object, what about the real world? How often can you really get away without behavioral guarantees?

Well, surprisingly often. I'm sure we both declare our variables as IList rather than ArrayList most of the time, for example? I tend to declare an IDbConnection rather than a SqlConnection, I'm sure you do too?

But then we have the cases when we do need a behavioral guarantee, and so we must bind to an implementation. In such a case, which we should strive to make rare in our designs, we must also "bite the bullet" and make the following, unfortunately unavoidable observation:

We can only get actual behavioral *guarantees* if we bind to a *sealed* class (even a non-sealed class but with only non-virtual members is not quite good enough, since constructors can always be overridden). And if we're really paranoid, we go on to observe that the sealed class must probably implement the interface directly (not extends any framework base classes) to avoid the fragile base class problem.

If we bind to any other type of concrete class, we make the rather weird decision to _pay the full price_ of binding to an implementation rather than an interface (we increase coupling, reducing reusability, robustness etc - plus we are suddenly bound to any bugs in that implementation!) but _not get the benefit_ in the form of behavioral guarantees.

What you get when binding to a non-sealed class is not so much a guarantee as...well, I'd call it the equivalent of a spoken agreement without witnesses compared to having a proper, signed contract (which is what binding to a sealed class would give you). It sometimes works, for sure, but all those times it doesn't the judge (in our case, the compiler) will just look at you and say "meh. shoulda gotten a real contact, you know".

(My comment is too long, continued..)

Mats Helander said...

"I'm probably too punchy to understand why you seem to argue so strenuously against "binding" to one of my concrete classes ... or a derivative thereof. Are you against inheritance altogether?"

I'm absolutely not against inheritance - quite the opposite, in fact (you should see my type hierarchies...) - but the point is: I'm against binding to a derivative of your classes only because of the fragile base class problem - but some would call that overkill/paranoia, so let's leave that aside!

So leaving that aside, I'm then not at all against binding to any of your *sealed* classes, since they *do* provide the behavioral guarantees that non-sealed classes totally miss out on. And, of course, in a sealed class none of the methods are virtual, which is why your whole problem will then go away :-)

"I wish to add only one more point. Mine is a living framework. We release a new version every 6 weeks."

And I should try to make one thing mush more clear: I'm not trying to convince you to make more methods virtual, or that you have extensibility problems (I'm sure you don't!). For example, I am a great fan of Template Method Pattern as a way to direct the efforts of extenders in a productive manner. What I'm saying is: When you publish classes that _clients are expected to bind to_: Seal them, and all your problems go away. You can still publish all your base classes etc. What I would imagine you would have to do in practice (not having seen your code base, but) is the following:

You have a class SomeThing that clients are binding to. Seal that class, but refactor any reusable parts from it to a new abstract SomeThingBase class. Extenders should then extend SomeThingBase rather than SomeThing, and their substitution will work:

- Wherever clients have bound to ISomeThing and thus _want_ such substitutability to work

but not where:

- Clients have bound to an implementation, and prefer behavioral guarantees over substitution - which is why these clients should be binding to the sealed class SomeThing.

I'm really hoping to be helpful (rather than a negative ranter), so...Is this making sense in the context of your framework? If not, why not?

/Mats

Anonymous said...

Reading this is like going back in time 10 years

David Nelson said...

I very much agree with Ward. What many who argue for virtual-by-default, especially those who have never built frameworks themselves, don't understand is that virtual methods *limit* extensibility. Don't believe me? Check out http://blogs.msdn.com/brada/archive/2003/07/29/50202.aspx:

"For example, we found that ArrayList item enumeration calls several virtual methods per each MoveNext and Current. Fixing those performance problems could (but probably doesn't) break user-defined implementations of virtual members on the ArrayList class that are dependent upon virtual method call order/frequency."

Additional analysis of the problem referred to can be found at http://blogs.msdn.com/ricom/archive/2005/08/26/performance-quiz-7-generics-improvements-and-costs-solution.aspx.

The primary problem with making methods virtual is that it invites consumers to take dependencies, not only on the behavior of the methods, but on the *order* and *frequency* in which they are called. This severely limits your ability to refactor and improve the class in the future. *That* is why it is so important to only make methods virtual that have explicitly been designed to be extensible: not to protect developers from themselves, but to protect the framework designer's own ability to evolve his framework to meet the needs of his customers.