Monday, June 01, 2009

This post is brought to you by the letters M and V and P. I mention that at the beginning to set the pre-requisites.

I woke up this morning, broke open our application, and am currently staring at this method in our presenter.

public void Start( )
{
    if ( !IsAuthorized( ) ) return;
    if ( !ValidateHeritage( ) ) return;

    if ( !IsCousin( ) ) return;
    if ( HasSiblings( ) ) return;
}

My first reaction to this was that it didn’t actually do anything. The way it’s currently set up, it appears to check a bunch of guard clauses and exit if they are met. But there isn’t any actual code being executed if the guard clauses aren’t met.

Digging deeper, I discovered that the guard clauses are actually doing more than guarding. They are calling methods on the View and otherwise changing state. For example, the IsCousin method:

public bool IsCousin( )
{
    if ( _user.AuntsAndUncles.Contains( _prospect.Mother ) 
         || _user.AuntsAndUncles.Contains( _prospect.Father ) )
    {
        _viabilityCount++;
        View.DisableSafetyChecks( );
        return true;
    }

    return false;
}

Clearly, this is ain’t your mother’s guard clause. It’s updating a local variable and doing some fanciness in the UI as well as guarding.

So maybe it’s just an issue with naming. Obviously, it’s not a guard clause, so maybe I could rename it to make that clearer. An obvious name isn’t leaping out but in any case, something feels wrong about having all these methods return a boolean.

An alternative I’m considering is separating the guard clauses from the actions:

public void Start()
{
    if ( IsAuthorized( ) == false )
    {
        SetNotAuthorized( );
        return;
    }
    if ( ValidHeritage( ) == false ) return;

    if ( IsCousin( ) == false ) return;

    if ( HasSiblings( ) == false ) return;
    
    IncreaseViability( );
    DisableSafetyChecks( );
}

Clearly, this is more verbose. But it looks better to me because the guard clauses don’t actually do anything except check the state of something. The actual flow of code is more obvious (isn’t it?).

Maybe the original method is fine and I’ve been reading too much on Command-Query Separation lately. Or maybe there’s an alternative. In which case, I’d love to hear it, even given the very limited context I’ve provided.

And in order to solicit as many opinions as possible, and because I know how much we all love to prove each other wrong, I’ll make the claim that my alternative is THE DEFINITIVE way of doing this.

Kyle the Reversed

Wednesday, December 10, 2008

There have been numerous posts about hiring the "right" people. One of the more ideal characteristics is a passion for the industry and for learning new techniques. But as I look at more and more code written by these zealots (and I don't mean that to be derogatory because I count myself as one of them) and because I have been thinking about brownfield applications lately for some reason, a kind of pattern seems to emerge in projects written by them.

The way to recognize this pattern is if you can tell what the developer has been learning or reading about from scanning various parts of the codebase. Here's a scenario:

---

New guy: How are we handling dependencies in the code?

Eager and enthusiastic veteran: Oh, it's awesome, man! At first, we were just newing shite up. Then I went to JP's course and he had this pretty cool hand-rolled IoC container so we started using that in the service layer. Eventually, we took the plunge and brought in Windsor. We don't use it everywhere but some of the newer presenters do. But then we saw the new stuff in StructureMap so this one part of the app is using that.

NG: So you have four different ways to get new instances of a class?

EaEV: Yeah, but don't worry your purty li'l head over it none. We'll be standardizing on one soon.

NG: Oh yeah? Which one?

EaEV: Well, it *was* going to be StructureMap but NInject looks pretty cool...

---

The point being: How do you handle implementing new techniques as you're learning them? You have a web application built by a team that still things they're coding in classic ASP and now you want to move it to an MVP pattern.

Naturally, for new screens, you can do this pretty easily. But as the code grows, you eventually get to a mishmash of new MVP screens and old code-smackdab-in-front screens that everyone treats like your disfigured, but still oddly attractive sister/cousin who's kept locked in the basement except during family reunions/speed-dating sessions.

What's worse, the problem is exacerbated as new techniques come in vogue and as new people are brought on to the project.

Also, for the most part, we usually don't want to go refactorin' just for the intellectual exercise. Most of the time, I like to move to a pattern or implementing some new technique on an as-needed basis. Which means you don't overhaul the entire application just because you're working in one area. If your admin section is working, I say leave it alone, despite your psychological issues with DataSets. (But at the first sign of bugs or new features in that area, bring in your hacksaw and have at 'er.)

In any case, it's cute little dichtomy. You certainly don't want to keep doing things the old and busted way but you are hesitant to fork part of the application down a new and inconsistent path.

And this isn't limited just to large projects. I'm experiencing it even know on an application I started earlier this year. An application, I might add, where I'm the sole developer. It's an MVC app with bits of Fluent NHibernate in it but not all the mappings have been migrated over yet.

The way to avoid this, of course, is to stop learning. But if you're one of those eager types, then let's explore some options. Unfortunately, all I've got are the thoughts in my head. And keep in mind, these are the same thoughts that evoked that little speed-dating analogy a few paragraphs up.

// TODO: Refactor this when the client stops reporting bugs and adding features

This is kind of the bare minimum you can do to at least acknowledge that some work needs to be done. It's well-intentioned but not very practical. Most of the time, we leave these comments because we want to cover our asses in case someone goes looking through the code and spontaneously goes blind. In any case, if the client isn't reporting bugs or adding features, that means no one is actually using the app.

The "Refactoring" Iteration

"That's it. I've had enough looking at this code. You, Mr. Project Manager Person, tell the client that we have deemed the application unpalatable and that we are holding it ransom until such time that it we feel warm and fuzzy about working with it again."

Don't need any limbs to count the number of times this has worked for me.

Leave it the way it is and strive for consistency

If you think you can get away with this, I applaud your ability to hire Type-B programmers.

Deal with it

This is probably my default approach. Hunker down and get your work done within the constraints laid out for you. Fix what you can, when you can. The application will morph. Try to control the shape and rate in which it does so. Knock of one head of the hydra at a time and hope three more don't grow in its place. It's probably not the most glamourous position to take but like any stance that involves lowering your standards, it's easier to attain your goals.

None of this strikes me as being all that encouraging, which is probably why this post lingered in my //TODO list for so long. I'm not sure there are any solutions to this other than to practice deep breathing exercises whenever you look at the code. Best I ever hope for is to provide value for my pay and try to learn something as I go.

Kyle the Acknowledged

Monday, October 27, 2008

I'm in the early stages of a new project with the dashing and debonair Brian Donahue. On the plus side, despite his misgivings, the codebase is in pretty phenomenal shape all things considered. SVN repository, automated build and tests, ActiveRecord, views reasonably separated into MVP. Even a half-finished CI process which is impressive for a one-man team. Like any codebase, it has its share of issues but all in all, I won't be using this as a sample app for a brownfield application.

On the minus side, he reads my blog so I'll have to be more subtle in the digs I make toward my workplace.

Anyway, one of the first things I like to do when I start looking through a steaming pile of excrement that someone calls "codebase" is check out the warnings that are lingering. The code had its share with the vast majority related to an upgrade to the latest version of Rhino Mocks but without changing CreateMock. This was an easy fix as a global search and replace from CreateMock to StrictMock got rid of over 80% of the errors.

The rest were a flurry of ReSharper refactorings until I got to the final four. Four assemblies reported the error: found conflicts between different versions of the same dependent assembly.

Double-clicking this error gives a helpful message offering to add a binding section to the assemblies app.config file. I say helpful seriously there because, while I had no intention of keeping that section in the app.config, at least it was able to tell me what the error message couldn't. Namely, which assembly was causing conflicts.

Or so I thought. The new app.config section seemed to indicate that there were NHibernate conflicts among various projects when I knew for a fact that we had exactly one copy of the NHibernate dll in the entire project.

Here's where Reflector comes in. More specifically, here's where it's extensibility comes in. Through the use of the Assembly Graph add-in, I was able to generate a picture of the dependencies. (Click on the image to the right for a bigger version.) As you can see, all referenes to NHibernate are going to the same assembly.

But as you can also see, there are three semi-related assemblies that aren't being referenced at all. Which means they can safely be removed from the various projects, yesno?

You probably think that's foreshadowing's cooler cousin, foreboding, but it's not. The answer is yes, I was able to remove them with no ill effects.

The reason I know there are no ill effects? Unit tests, baby! None o' this "let's take a stroll through the application and see if it still works" nonsense for me*. Remove the reference, run through the build process and watch the magic happen.

The reason this is important: I'm pretty sure this add-in doesn't account for dynamically loaded assemblies. And while I'm also pretty sure we would have no reason to dynamically load some peripheral NHibernate assemblies, it's nice to have a suite of unit tests backing up that assertion.

So to re-cap:

• Unit tests good
• Automated builds good
• Reflector good
• Assembly Graph add-in good
• Subversion good (I haven't provided explicit evidence of this here but it's worth mentioning.)

Kyle the Reflectively Assembled

*Brian, if you're reading, I'm lying. I did actually go through each section and make sure it's working. But back me up if anyone asks.

Monday, October 20, 2008

I'm preparing my presentations for DevTeach and have come to realize that there is a price to pay for creativity. Here are the abstracts for three of my presentations.

---

Guerrilla Refactoring

Greetings, comrades. Welcome to the resistance! Our quarry today is a shamefully designed application that grows fat with duplication and crusty with hard-codedness. It feeds on itself behind a shield of corporate deadlines, each one stricter than the last. The cowardly code mocks us with cries of "We don't have time! We don't have time!" while features are grafted onto the application haphazardly with no thought to future reform!

But fear not! We have studied our enemy and have altered our attack plan accordingly. No more frontal assaults. We must liberate the code in focused skirmishes from its bourgeois oppressors! Patiently, we shall advance, tightening our grip in an ever-widening net of unit tests as we hunt down bugs like the dogs they are!

So join me, brethren and sistren! With our allies, the Design Pattern Legionnaires and the Dependency Liberation Front, we will train you to take your rightful place in the movement toward a better world, where features are not divided into tasks that are meted out to the "database gal" and the "guy that does the UI because he has PhotoShop installed". Rather, they will be fully and utterly DEMOLISHED by fully armed, multi-functional, and domain-inspired programmers full of KNOWLEDGE AND FURY!!!1!!!

Viva La Revolucion!

(adapted, of course, from my original post on the subject)

---

Implementing a Brownfield Ecosystem: A Cultural Extravaganza

So you missed Folklorama again, eh? Of course, you did. It's in Winnipeg. Well, fear not, coders! In this session, we'll throw so much culture at you, you'll think you're back in high school biology.

A very important aspect of Brownfield applications is shifting the culture of your team. This starts with your project's ecosystem. Many developers in Brownfield applications don't even realize how many hoops they jump through during the course of a day to do simple things like checking in code and building the application.

In this session, we'll implement a full-fledged ecosystem for a Brownfield application. Working from the perspective of a single developer in a team, we'll cover topics such as the structure of your version control system, your check-in process, automated builds, and continuous integration. In the process, we'll talk about common pain points and common areas of friction that we can overcome with a few simple tool choices and a couple of mindset tweaks.

I'll be getting by with a little help from my friends as we simulate a team environment using well-placed and impeccably groomed plants in the audience who will act as other developers in the team, some working with me, others not so much.

---

Adding automated tests to an existing codebase

It may seem like heresy but there are actual real-live applications out there in the world that don't have unit tests! No, really! And I know it sounds crazy, but you could be brought on to one of these applications and be tasked with adding automated testing to them. 

Rare as these circumstances may be, it'll help to be prepared for them. Because it won't be as fun as you are probably imagining right now. For example, how do you automate a test for a web form that connects to a database, displays a form, then posts that data back to the database?

In this session, we'll look at techniques for integrating tests into an existing codebase that has never had them before. When do you write tests for existing code? How do you test a mammoth method/class? How do you know if you aren't breaking anything else? For answers to all these questions and more, read Michael Feathers' Working Effectively with Legacy Code attend this session.

 

---

You see my dilemma. I wrote these in a fit of imagination without stopping to wonder how I would differentiate one from the other. For example, how do I talk about refactoring without mentioning automated tests? How do I implement a brownfield ecosystem without doing same?

 

In the end, I believe there will be overlap among the presentations. Indeed, I will probably use the same sample application for all three. I've justified it in my head by saying that the overlap is probably a good thing to help people see the interaction 'twixt the various concepts. Plus, it means I can focus on a single element in, say, the refactoring presentation and leave some of the details to the automated tests presentation.

In any case, here is some clarification on the goal of each presentation:

Guerrilla Refactoring: How can you refactor an application for maintainability when there is no explicit buy-in from the PM and/or the business (and/or the rest of the development team)?

Implementing a Brownfield Ecosystem: A walk-through on adding source control, CI, and automated testing to an existing application. I'm planning to use plants in the audience to simulate a team environment to demonstrate what happens to you personally when others don't play by the rules. The "cultural" part comes because this is very much an exercise in changing your team's development culture. It was originally intended as a practical follow-up to Donald's Parachuting into a Brownfield Application presentation but I notice he's not doing it this time around.

Adding automated tests to an existing codebase: Here, we'll dive deeper into automated testing for an existing app. It will be sort of a continuation of the brownfield presentation which will add the testing framework but not go any further. Admittedly, it will borrow heavily from Feathers' book but I see that as a good thing.

For completeness, the fourth session I'm doing is on TDD for MVC applications. This one is more disparate than the others and should be a fun one as well, even if the abstract for it is the most boring of the four.

Kyle the Synergistic

Saturday, January 05, 2008

The "How to" is gone from the title to be resurrected at appropriate intervals. In my last post, it was brought up in the comments that it was, and I quote, "naff". Wikipedia helped me with a definition which confused me a little because "cheesy" and "tacky" were the exact adjectives I had in mind when I started using the device some eight months ago in a fit of Kubrick-love (and looking over that link: yoy, I gotta stop writing my thoughts down).

No matter, the Hillbilly is nothing if not easily swayed by anonymous commenters. So the "How to" will be limited to posts on Guerrilla Factoring and Code Coverage Nazis and the like. Back to the show!

The topic of the day is MVP and/or MVC within an MDI application and the blog will act more as a sounding board this time around because I'm having trouble putting my thoughts into something concrete I can type. It probably could also apply to a master form that uses MVP/MVC and which contains several user controls that use the same pattern. If it does, that's not my fault.

Here's the scenario: The main container form has a menu item that says Open which will open the currently selected item as an MDI child. And ummm....I suppose that's it really now that I think about it.

At the moment, both the main form and the child forms are monolithic hydra-like beasts forms that contain all presentation logic and a good chunk of business logic. And so, being a diligent hillbilly, I'm adding a presenter to the main form. Clicking on Open passes the call on to the presenter. Which does....?

My first reaction was that it would create the child forms (which I'll call Estimates, mostly because that's what they are) and maintain them. This would be the easiest implementation because it would involve cutting and pasting code without thinking about it too much. But it feels funky to be manipulating Windows.Form objects in a presenter. Plus how would one interact with them? If I wanted to, for example, save an Estimate, would I need to call a Save method on the Estimate view which would delegate to its presenter? Or do I maintain a list of Estimate presenters instead and make the calls directly on them? And so on, and so forth.

Then I took a step back (tripping over the sofa) and thought "what's the model here?" The main form is generally just a window manager, I think, and its model should reflect that. So rather than having the presenter do all the form management, it should delegate to a WindowManager. Which sounds to me like it would be a UI-based model but a model nonetheless.

Jeremy Miller's StoryTeller has a similar UI metaphor (hierarchical view on the left, tabs on the right) and provided some guidance in this regard. It threw me at first with the main form being a mere 68 lines long (compared with ours at 2344 lines). In it, he passes everything off to an ApplicationController, which is managed through his hierarchical view. The ApplicationController has all necessary dependencies injected into it and it handles opening screens, mostly by delegating them on to the panel that is going to house them.

This kind of jibes with the conclusion I had eventually come to above. And this post was a couple of days in the making so don't be assumin' all of this is off the top of my head.

So in summary: The model for the main form of an MDI application is a window manager/controller that knows how to open other windows.

Thanks for listenin' and if the end result makes me look good, I'll come back and let you know how it turned out.

Kyle the Round-about

Monday, December 24, 2007

Welcome back, revolutionaries! We've had a small reprieve from the fight to let our early anarchy settle in and---....y'know what. The revolutionary jargon is getting too hard to write and the refactorings more involved so I'll just throw out a "The revolution will be blogged!" and go back to bein' a hillbilly, iffen it's all the same to you.

Today I'm throwing myself at the data access layer and the database. The data access layer is an easy target just because it exists and we've long since switched to NHibernate for our data access in other applications. So the refactoring there will entail ripping it out mostly. Along with most of the stored procedures which haven't inspired me with a lot of confidence ever since I found three of them that refer to tables that don't exist.

One of the pain points I've encountered in dealing with this particular database is the different schemas it uses. For those of you not familiar with the concept (as I wasn't until about a week ago), most databases will use the default schema, which is usually dbo, probably for database owner but don't quote a humble hillbilly. In this case, almost all the tables were created under a different schema. For example, the users table belongs to a Admin schema and the FieldList table belongs to a Dictionary schema.

To create the table under a different schema:

CREATE SCHEMA [Admin]

CREATE TABLE [Admin].[tbUsers]
(
[UserID] [int] NOT NULL IDENTITY(1, 1),
[NetworkUserName] [varchar] (255) NULL,
[RoleID] [int] NOT NULL,
[PreferredPlaidColour] [varchar] (50) NULL
)

After this, you'll need to qualify the table in any queries you make to it:

SELECT * FROM [Admin].[tbUsers]

I'm certain there are advantages to using schemas. Security comes to mind as I'm betting you can apply security rules to a schema to grant/limit access to it. And they probably appeal to DBAs that like to keep things pseudo-organized.

For our case, it was overkill and a surprisingly potent distraction when we had to query the database. This was the only application that used them and it wasn't exactly a high-profile, high-security type application. It's used by one app, ours, and you either have access to the app or you don't. And they weren't even used consistently. There were two tables using the default dbo schema, at least one of which shouldn't have been.

So after talking with some developers who worked on the previous version and confirming that they weren't being used in a meaningful way, we set about removing them. Which is much easier than one might think:

ALTER SCHEMA dbo
TRANSFER Admin.tbUsers

I do have to send a shout-out to DataDude, which you may know by it's official name (and I'm going by memory so it may be off a bit): Microsoft Visual Studio Team

Partner Coding System for Professional Licensed Database Practioners and Ornothologists, the Spongebob Squarepants Edition.

This nifty little VS add-in/utility/whole entire version of the product has been a tremendous help during this refactoring. It has a Schema Compare feature that I used to generate the initial scripts to create the database and a Data Compare feature I used to generate test data for our QA environment.

But a nice feature I wasn't expecting: after I changed our CREATE TABLE scripts to use the new default schema, Visual Studio immediately popped up a pile of clickable warnings indicating stored procedures, foreign key scripts, etc, that were no longer valid because the name of the table had changed. Not quite as clean as a ReSharper Rename, mind you, but better than hunting through scripts manually and inevitably missing some.

It was also DataDude that discovered the three stored procedures referencing non-existent tables.

The net result: a database more in tune with its sister databases and one that inspires far less cursing from developers who are prone to typing in SQL statements without the schema.

Kyle the Databound

Saturday, December 15, 2007

Fear not, brethren and...uhhh....sistren. The resistance is still alive. But the revolutionary jargon will get in the way here so I'm reverting to a more traditional means of expression.

Among the things we've implemented as part of our project clean-up is the aptly-named ClassTester. It's a neat little tool that takes some of the pain out of routine testing of constructors and properties by, for example, setting a property then checking to make sure you get the same value back from the getter. Here's some sample syntax for testing the properties of an object:

    ClassTester tester = new ClassTester(new myObject());
    tester.TestProperties();

Now, hillbillies are not what you might call amenable to writing boilerplate code for every single class in their domain. Leads to some problems you can only imagine in your nightmares. So in an attempt to avoid duplication, I decided instead to walk through all the classes in the assembly:

       [Test]
1       public void Test_general_constructors_and_properties()
2       {
3           Assembly assembly = Assembly.GetAssembly(typeof (Still));
4           Type[] types = assembly.GetTypes();
5           foreach (Type type in types)
6           {
7               if ( type.IsClass 
8                    && !type.IsAbstract 
9                    && type.Namespace == "Suvius.Applications.HoochDeriver.Domain" ))
10              {
11                  ClassTester.TestConstructors(type, false);
12                  object item = assembly.CreateInstance(type.FullName);
13                  ClassTester tester = new ClassTester(item);
14
15                  tester.TestProperties();
16              }
17          }
18      }

Some points of interest:

• I could use any domain object in line 3. I'm interested only in getting the assembly that houses all the domain objects
• Line 8: The ClassTester doesn't much like abstract classes
• Line 9: This line may seem redundant but it is necessary because of the way I run my builds (which, credit where it's due, I pilfered from JP Boodhoo). Namely, in the build file, we compile all .cs files into a single assembly and run the tests against it. Which means line 3, will retrieve the assembly containing (almost) every class in the application. Compare this with compiling in the IDE where all domain classes are in one assembly by themselves. So running this test from the IDE, where it will compile the assemblies separately, we don't need the check for the namespace. Running it in the build file however, we do.

There are in fact two separate tests here, one for constructors and one for properties. If you want to argue that these should be separate tests, I'll agree. If you want to argue against that, I'll agree too. That's how much I care about it.

Anyway, this code is nice in theory. At present, the tester handles only very routine cases. If there are any validation rules, you'll get validation errors. For example, if you have a Percent property where you throw an exception if the value is not 'twixt 0 and 100, the ClassTester will throw this exception most of the time because it generates a random number to set the properties and doesn't know about your little rules. The ClassTester does allow you to ignore individual properties though, which is useful when testing classes individually.

The second problem with looping through every class is that not every class can be tested, even with no validation rules. In our case, we had some classes that were inheriting from CollectionBase (did I mention it was ported over from .NET 1.1?) and the ClassTester failed on all of these because of the Capacity property (which I didn't know existed).

The end result is that we had to include a list of classes to ignore from the domain and make sure they were tested separately through more conventional methods. Whether or not this is worth it depends on how many classes you need to ignore. In our case, it was only the collection classes which have since been removed anyway because of the generic collections in .NET 2.0.

And to prove it was all worthwhile, the ClassTester found and located no less than three bugs in our getters and setters, each one a variation on this theme:

    public decimal AmountOfHopsToAdd
    {
        get
        {
            if (DandelionBase != null)
            {
                // Return some complicated formulaic value based on DandelionBase

            }
            else return 0.0M;
        }
        set { _amountOfHopsToAdd = value; }
    }

In this specific case, the setter is all but useless since _amountOfHopsToAdd is never actually used anywhere. The other examples gave similar fodder for pondering and our code is shinier and happier for it.

Kyle the Testable

Friday, December 14, 2007

Here's a little sidebar that I bet you didn't know. Actually, that was probably more accurate at my old site where my readership measured on a more, shall we say, intimate, scale.

Anyway, I recently converted a project from NUnit to MbUnit just because I felt like it. (Oh, I'm kidding. I have reasons. Boring but valid ones. So don't sic the alt.net police on me.) In the past, that has involved adding the assembly reference for MbUnit, then performing a global search and replace of the appropriate using statement. Perhaps cleaning up the different Assert syntax but in general, it's a pretty painless process.

This time, I got null reference errors in all tests in several class and to make a long story short (don't get used to it, I don't do it often), I traced it to the SetUp method:

    [ SetUp ]
    protected void SetUp( )
    {
        // Do stuff
    }

Notice that it is protected, which apparently makes it all but invisible to MbUnit. I switched it to public and my world is fully tested again.

Initially, I thought it might be worthy of a bug report. But here is kind of what I typed out:

Hey dudes! You got yerself a fine 'n dandy little product here. But I got meself some tests here someone was nice enough to write out for me in NUnit and when I converted them to MbUnit, they done gone failed on me 'cause the SetUp method is protected even though... the tests themselves are...ummm... y'know...public...

Here's where I started to feel a little silly and considered that maybe the MbUnit folks knew what they're doing when they wrote the product (although I wouldn't mind it handling nullable types a little differently).

So I guess I don't really make this long story all that shorter. The Hillbilly is nothing if not contradictory.

Kyle the Taciturn

Friday, December 14, 2007

Greetings brothers and sisters and solidarity to you. The revolution is off to a rousing start. We have integrated continuously and Comrade Marc Davis has won a key early victory by excising the putrid web service that so fiendishly guarded our data access layer.

As for myself, I have just returned from a bloody battle with the enemy and have come away battered but triumphant. It was a great blow to the despots of uncohesiveness!

Our wily foes tried to gain the upper hand early by striking when our defenses were low. After opening a seemingly benign file, we were awash, AWASH I tell you!, in a veritable SEA OF ORANGE running along the right side.

I stared dumbfounded at the awesomeness of the task before me. "How can this be?" asks I, "this is no mere autogenerated class? This class was typed by hand. Clearly by the devil himself!" No matter, with a cry of "ONWARD!!! TO BE RELEASED FROM UNDER THE OPPRESSIVE THUMBS OF THE HIGHLY-COUPLED SWINE-PIGS!!" I leap headlong into the fray.

My battle begins and I become the Comment Warrior. It is a bold move because of the sheer numbers. <summary> tags that hold no relevance, <param> tags for non-existent parameters. No comment survives. Void methods have been tagged with <returns>Nothing</returns>, a move which stuns me at first but I quickly recover. After several skirmishes, I no longer notice the empty lines of History: in the <remarks> tags as I go about decapitating the methods in the name of maintainability.

Soon, I see an opening! Find & Replace can take a regular expression! After a brief two-day refresher of the syntax, the first phase is mine in one fell swoop of replacing the following expression with an empty string:

^:b*///.*\n

With that, the opposing army is almost 1000 less strong. But the enemy is quick and responds with waves upon waves upon waves of:

    catch
    {
        throw;
    }

along with their slightly bigger pawns*:

    catch ( Exception ex )
    {
        throw ex;
    }

These blocks are thrown at me from *every single method*. Some even have finally calls that set object references to null. Clearly, this army's overlords were trained in the style of Crouching Tiger/Visual Basic from the late 20th century.

Luckily, they are easily picked up with the Blade of Alt-Enter from my ReSharper arsenal. But after untold hours, my fingers become weary. And my Mace of Shift-Ctrl-Alt-F fails me when it is unable to reformat the 6000+ soldiers remaining in this class. But I risk a glance to the right and allow myself a brief bout of euphoria as the orange gives way to the dull gray of victory.

By this time, the battle is all but won. I drive a final blow by deleting all regions (regular expressions: ^:b*\#region.*\n and ^:b*\#endregion.*\n) and with a flourish of Ctrl+Alt+O, all references are properly qualified for the final victory!

Resistance is NOT futile!

Kyle the Understated

* Comrades Dave Woods and James Kovacs both have posts on why this variation is particularly heinous

Thursday, December 13, 2007

Have started a new project at the current contract and I'm muy excited about it. It's a "legacy" .NET 1.1 app that was upgraded to 2.0 by running the wizard, then left alone. It has a web service created by an architect who appears to have worked with Jeremy Miller before. It returns datasets. Lots of them. There's a DataDude project created in some CTP version that can't be upgraded. There is no CI process.

I discovered most of this without cracking open the code. I have since done so and analogously speaking, it is clearly the result of the type of relationship we hillbillies are *very* familiar with.

I'm pretty pumped to dive into it, actually. The CI process is mostly done. The tests are running regularly (2.5% code coverage, woohoo!). Now it's time to start the grand-scale refactoring.

First, however, we need to go to the customer and tell him, "The architecture of this app is not unlike something akin to roadkill. It compiles only during the waxing phase of the moon and the fact that it runs at all has restored my faith in God. I know you've been using it relatively successfully for several months but we'll need several tens of thousands of dollars from you to refactor large portions of it. When we're done, there will be no discernible difference to the application from your perspective but we in the IT department will sure feel better about ourselves."

Now I've never been able to say "discernible" out loud convincingly and I'm sure as heck not going to imply that I believe in a higher power so obviously, this isn't going to work for me. Which leads me to a tactic I've become quite fond of: Guerrilla Refactoring

Guerrilla refactoring requires some unconventional tactics. Like in guerrilla warfare, I'll need to ambush the code often. Come at it from an angle it's not expecting. Do quick, focused skirmishes and get out again before anyone notices (except the code which will be grateful to be liberated from its bourgeois oppressors).

Subterfuge and guile will be my allies. "How long will it take to implement the 'sync' function? Four hours, minimum. Change the layout of the form? A good day of coding. You need the text changed on this label? I'd say....three full days. Not including writing tests to expose the bug."

I will bear the brunt of their skeptical, leering eyes for the sake of the common good. No longer will we toil under the totalitarian regime of logic-laden views and hand-rolled data access code. Their armies will *tremble* in fear in the face of my legion of Freedom Fighter Presenters! And the fascists will COWER as I slash at them with my Sword of NHibernate!! JOIN ME, brothers (and to a lesser extent, sisters), as we stamp out the Dictatorship of Duplication and drive a *hammer* through the very heart of hardcodedness!!1!!1!!!!

VIVA LA REVOLUCION!!!!

Kyle the Pacifist