Reflections

Today, I want to tell you about a startling revelation regarding the effects of pair-programming on your code’s quality. I mean, sure, there’s tons of literature out there raving about pair-programming and its benefits on code quality, but when it comes right down to it, you always have to explain why it’s cheaper to have two programmers working on a problem instead of just one.

Okay, sure, there’re the obvious reasons, such as breaking in the new guy, establishing collective code-ownership, creating a highly complex piece of software or security critical feature. But how do you argue for the benefit of pair-programming when you’re writing more or less run-of-the-mill code that’s just supposed to be rather bug-free and reasonably well refactored, and you have a team of great programmers already attuned to the problem domain?

Well, there’s at least one scenario, and that’s spotting design flaws and more importantly, finding a solution to them. See, here I was, happily sketch a design and handing it off to a colleague for implementation, then doing regular reviews. Eventually, there was a point where an alteration was necessary. At the time, the proposed solution seemed reasonable. I probably wasn’t perfectly happy about it, but it appeared to be well refactored and following the principal of separation of concerns, so I just went with the flow because it got the problem solved.

Obviously, it wasn’t the right solution, or I wouldn’t be blogging about it now. The big surprise was the way I realized not just the problems in it—aside from a bug-report—but also the solution: I was refactoring to fix the bug.

You see, that’s the big difference between doing a review and sitting right there next to your partner in crime. In one scenario, you’re a passive observer, highlighting obvious troublesome spots and talking about established facts. In the other, you’re actively involved in massaging the code, feeling the awkwardness, and hopefully sparking that part of your brain that’s responsible for solving the big mysterious of our time.

Unfortunately, all things considered, this example perfectly establishes why being actively involved in the process leads to better results, but it still doesn’t make a good case for twisting the review-knob all the way to 100%.

Why, you ask? Well, writing that part of the library took weeks. Fixing the design problem in an intense refactoring session took two days.

So, bottom line: Two guys working on two parts of the system can still be more profitable than two guys sitting right next to each other and sharing a keyboard. But you have to be aware that the design will suffer from it. But if you have a great team, the odds that you will come out ahead of the game are still better than even.

On a parting note: Right now I’m musing whether or not turning review sessions into refactoring sessions might be a way to mitigate this problem a little bit more…

Michael

In my last blog post, I talked about the pain a misused singleton can introduce into your development lifecycle, particularly if you’re doing test-driven development. But what can you do when the singleton in question isn’t under your control, but provided by the framework? And to top it off, it’s not even fitted with an interface so you can’t mock it. Oh, and in order to instantiate it and use it in your test fixture, you need to resort to reflection.

Well, for years my answer had been to bite the bullet. Accept that the framework isn’t suited for a test-first approach. And write reflection-based helpers that allow me to test my most-critical components, at least. Any guesses which real world example I’m talking about?

Enter HttpContext.Current, System.Web.UI.Page, and just basically the entire ASP.NET WebForms stack.

Let me start by listing a couple of scenarios where this is an issue:

•  Developing an HTTP handler
  A simple handler doesn’t even have a user interface, and still, as soon as you need to interact with the request, the response, the session, etc, you’re back to fighting in the trenches. There are, of course, ways to get at least some test coverage, but eventually you have to accept that there is a place where no unit test will dare to go.
  
• Developing a custom web control
  In addition to dealing with the HttpContext, now you also have to deal with the page reference, the page-lifecycle, protected and internal methods you’d have to call from your tests, etc. Not fun. Not fun at all.
  
• Developing a UserControl or a Page
  This is where it get’s *really* interesting—or ugly—and I’m not even going to begin listing the problems you face when trying to unit test those monsters. To put it simply, there’s a very good reason why Microsoft developed ASP.NET MVC.

Okay, but what about the first two scenarios? Well, the answer to testing an HTTP handler is surprisingly simple—just introduce a mockable layer of abstraction between your code and the HttpContext. So, why did it take me till summer 2008 to come up with it? Mainly because I had scruples doing something this radical. And no time. See, if you want to add an abstraction for HttpContext, you need to provide delegation for all members. You need to test it. And you need to provide abstractions for all dependent types as well, e.g. HttpRequest, HttpSessionState, etc. Plus documentation, because you don’t want to expose the users of your types to the bare metal APIs.

So, what changed in 2008? Easy answer: I realized the infinitive potential of ReSharper’s ‘Extract Interface’ and ‘Delegate Members’ refactorings. Now all I had to do was create a new type, add a field for my wrapped instance, execute two refactorings with ReSharper, and I had my layer of abstraction. Took me all of thirty minutes or so. Okay, I didn’t write tests for it, but I trust ReSharper not to muck up the code generation. Add a bit of plumbing and some null checks, and the entire HttpContext infrastructure is now mockable.

Of course, a few months later we started to depend on .NET Framework 3.5 SP1 in re-motion, and I realized that the old saying about inventions happening at multiple places independently when the time is right still held true. Microsoft had moved the System.Web.Abstractions assembly from the ASP.NET MVC Preview into the core framework and lo and behold, Phil Haack and his gang chose the same approach for ASP.NET MVC’s testability. The only difference is that the official stack uses abstract base classes and doesn’t expose the wrapped instance. In .NET 4.0, those types will actually get moved into the regular System.Web assembly. And in case you’re wondering, my implementation is already earmarked for the big ‘Safe Delete’ refactoring ;)

This leaves me with control development. Here, you have to distinguish between two basic aspects. One is logic that depends on the correct invocation of methods according to the page lifecycle. In this case, the only testable approach is to gut the control, create some controller classes and generally follow a divide and conquer approach. Once you are able to do this, all that’s left of the control is a façade, the properties, and the design-time support.

Much more meatier is the stuff that happens when the control is interacting with the outside, e.g. registering scripts, rendering its contents, etc.

So, ever checked out the mechanism provided by ASP.NET for script registration? The API you’re looking for is the ClientScriptManager exposed by the property ClientScript on Page. It was introduced with .NET 2.0, replacing the previously used methods exposed directly on the page. Then, a year or so later, along came the ASP.NET AJAX Extensions, back then a separate download and located in the assembly System.Web.Extensions. Now, if you want to use asynchronous postbacks via the UpdatePanel, you have to use static methods on the ScriptManager to register your scripts. Suffice to say, I was not a happy camper.

This time, the solution was a tad more complicated and intrusive, but it integrated well with a concept introduced into our web-stack back in 2004—interfaces for Control and Page. The original reasoning behind it was to give projects the option of using a custom or third-party layer-super-type and still integrate their pages with re-call, re-motion’s control flow architecture for ASP.NET WebForms. In order to achieve this, we merely copied the signatures of all public members into the interface, derived our own interfaces (e.g. IWxePage) and that was it. Of course, this also meant exposing the ClientScriptManager and the HttpContext as is.

The implementation was simple and straight forward, and it served it’s designated purpose. But it also failed to open the door to do actual test-first development because I couldn’t just mock the Page property on a control, nor the Context property on the page returned by said Page property, not to mention making it possible to test my script registration logic. All these requirements called for a much larger refactoring.

To make a long story short, I introduced an interface and a wrapper for ClientScriptManager, merged the ScriptManager’s (relevant) API into this interface and changed the type of the ClientScript property on re-motion’s page interface (IPage). Same goes for the Context property and the Page property. And the result of this effort is that I’m now able to actually expect the registration of a specific script when I implement a new feature. The first benefactor of this approach are the web components implemented in re-vision, our document management system.

I will do a separate blog post some time in the future that shows how to leverage our web-stack, but for now the conclusio is that it is now possible to write unit tests for your extensions to re-motion’s web-stack.

Michael

Okay, it’s 2010, and my New Year resolution is to fill this blog with life. Luckily for me, I have the perfect candidate desperately deserving some spotlight – the singleton pattern.

“The singleton, seriously?” you might ask. After all, it’s probably the most widely known pattern out there. For instance, let’s take a question right out of the classic job interview for a developer position. “What patterns do you know?” Care to make a guess which pattern gets picked the most? Yep, you’re right, it’s the singleton. And you know what else? It’s even a valid answer. The singleton pattern is listed by the GoF.

So, why did I choose it to get picked on? Easy enough to answer – it’s also the pattern that’s causing the most trouble when it comes to application design. Don’t get me wrong, there are tons of anti-patterns that can cause even greater harm in an application, but programs suffering from really bad code typically don’t employ too many of the patterns described by Gamma et al.

But enough of that. Let’s start chipping away at our global variables instead. Oops, I meant singletons! After all, there are no global variables in our nice, clean, object oriented world. Or are there?

Okay, I guess, that’s what you could call a loaded question. But why is a global variable a bad thing? It’s accessible from every scope. It’s perfect when different parts of the application need to access the same shared state. And I could list so many scenarios where a singleton makes the code really simple. All it takes is a reference to the instance and I’m golden.

Or not.

To illustrate my point, I’ll take one of the most common use cases for the singleton pattern – aside from the System.Web.HttpContext and the application configuration – the current User. That’s information I need all the time when I’m building an application that requires authentication, enforces security, logs changes to the business objects, etc.

A quick disclaimer in between: Yes, I’m aware that ‘singleton’ refers to there being just a single instance of a specific type, and there’s hardly just one User or HttpContext in a web application. That’s why I’m using the term ‘well-known instance’ most of the time. But for the sake of this blog-post, they aren’t all that different and ‘singleton’ is so much easier to type and read.

So, here I am, happily building a little booking application. One requirement is that each Order must be associated with the User who created the Order. Easy enough to do; I just assign the value from User.Current to the Order.CreatedBy field inside the Order’s constructor, and I’m done. Next, I launch the application, log in, create a new Order, and save it. I can easily verify the correct behavior through the application’s UI, via the debugger, or by checking the database.

Hmm… I know that I’m missing something. Oh, yes, the unit tests. Stupid me! I have to write a test first, and then the code that’s doing the heavy lifting. Let’s see… I get a new Order from the OrderFactory, assert that it is correctly initialized and that CreatedBy is set. Wait, that’s strange… The assertion fails; CreatedBy is still null. Oh, right, I forgot about User.Current. I set it, and the test is green.

Now that the Order class is done, I can start using it all over the place. After all, it’s one of the core types of my business domain. For instance, I want to check that each User can only select his own Orders. So, I’m creating two Users, Alice and Bob, and create a couple of orders for each, remembering to change the value of User.Current halfway through the test setup…

Anyone starting to feel the pain? Don’t be shy, raise your hand. I’ll even go first. And before I start sketching a scenario where each and every one of my domain tests requires a correctly set current User – and potentially triggering all sorts of nasty flashbacks of uprooting such evil – I’ll head on to the message-part of this post:

Don’t use a singleton just because you need a specific instance all over the place. And if you really do have a use case for it, think about it one more time and then don’t use a singleton.

Why? Because it flat out ruins your code’s testability. For each and every singleton your system under test depends on, you need to add setup and teardown logic, always making sure that your tests don’t have side effects. You don’t want to do that. It’s painstaking. It usually makes your test-runtime skyrocket. It is an external dependency. And it can drive the next guy working on this code nuts. Btw, chances are that’s going to be the you from a couple of weeks in the future.

Does this mean I don’t use singletons? No, not really. Especially when I’m working on re-motion, it’s usually the way to go. And by ‘usually’, I mean about a dozen config classes, holding the deserialized configuration or reflection-based metadata. Neither tends to change at runtime in applications built on top of re-motion, but forcing the users of re-motion to pass those instances around when creating objects, well, that would make the API really hard to use.

If that sounds like it’s okay to use singletons in a framework but not in an application, well, that’s because it is. Or at least, that’s what I’m telling myself to stop from obsessing over designs I chose before I saw the havoc a misused singleton could wreck out there in the wild.

But what about applications? How can you get rid of your configuration singletons? Well, you can’t. You still want to cache the config once it’s deserialized, but you can pass it into the constructor, thus removing the dependency on the singleton instance. Same goes for current User and other pieces of data used far and wide in the application.

Of course, as soon as you start passing this data around via constructor arguments, you find yourself thinking about object composition, inversion of control, factories and strategies, and generally structuring your code in such a way that you only have a few, carefully selected entry points. But that’s stuff for another blog post.

Michael

What? I’m a developer and that’s how we usually start out, isn’t it? But since most of my readers – that’s you, btw. – are developers, too, that’s not going to be news to you. So, let’s try it this way:

Hi, my name is Michael Ketting and I’m part of the the re-motion team, just like Fabian Schmied and Stefan Wenig. Okay, okay, that’s not a surprise either, given the domain-name of this blog. What you just might be interested in, is what this blog is about and why I even started this.

Well, the second part of the question is going to be easy – it’s been a year since Fabian started out, and it had almost been a year before that that the team-blog went live with an article about our lang.net participation, so I figured it was high-time. And no, I have no idea who’s going to show up around here in a year from now.

Good, now for the harder part, namely finding reasons for you to come back here, or maybe even use the nice, little ‘create RSS feed’ icon in your browser.

One reason could be that you’re interested in the rest of what re-motion has to offer, besides re-linq and mixins. No, I’m not going to copy and paste the xml-doc here, but a blog is the perfect place for showcasing some of the grittier bits and pieces. You know, things like how to make use of the extensibility points that I painstakingly wedged into the re-motion source code over the past five years. Yes, the framework really is that old. And no, I can’t believe that it just was about a 100 types small when I started out at rubicon in November 2004.

Another reason could be to read my reflections on all sorts of things having to do with producing a high-quality code base. Not that a lot of it will be shockingly new if you’re familiar with the works of Beck, Fowler, et al. But hey, before I harp about yet another anti-pattern I’ve stumbled across, I can just as easily publish a blog post and maybe – just maybe – prevent another singleton from ruining some poor guy’s day. Or night. Literally and just before the go-live.

So, if you enjoy reading a blog where technical content meets the occasional snarky aside, this one might be for you.

Till the next post,

Michael