A Day In The Lyf

…the lyf so short, the craft so longe to lerne

Archive for June 2007

Getting it to work is not the only goal

I’ve had the good fortune of working with some really bad developers in the past. I’m sure the content of that sentence suggests some sarcasm to you; I assure you that I mean none. At one point, I myself was one such developer and didn’t realize it. This seems to be a common pattern—bad developers don’t know that they’re bad, and quite often they think they’re really good. Evidently, the pattern isn’t restricted to software developers (see this study for example).

Working with bad developers has made me a better one by seeing the results of bad practices. One of the things I now realize, for instance, is that getting something to work isn’t the only goal when working on a task. You also have to make sure that it will keep working. If you say you’re done after whatever you’re working on appears to work, you’re only doing half of your job. And probably, the easier half.

Making sure it will keep working means different things to different people. In my view, it means making sure the code is as simple as possible and getting it under a set of automated tests so that future developers aren’t afraid to change it. Too often I’ve seen bad developers point the finger at someone who has the gall to change “their” code when the change doesn’t work. “It’s complicated,” being one of the most immature and insulting complaints, as if it’s ok to erect a solid barrier around code you’ve written because you’re the only one smart enough to understand it. In my experience, that attitude represents a lack of aesthetics when it comes to coding. It’s the result of saying you’re done when you get it to work, not when it’s simple. Simple Ain’t Easy. It’s the result of thinking that testing is something that only those who aren’t smart enough to be developers do.

The desire to stop when you get something working isn’t a problem that only bad developers have; it’s just highlighted more against a backdrop of bad developers. But it’s a discipline that even good developers break from time to time.

The team I’m currently working with is easily the most competent group of developers I’ve ever worked with. We’re actually pretty decent at trying to keep things simple and get as much of our code tested as we know how. But there’s some code which we just haven’t found good ways to easily test yet. And unfortunately, we still break code changing it.

In my opinion, being afraid to change the code isn’t the answer. In fact, being afraid to change the code seems like the worst possible solution. It’s surrendering. It’s giving up on the aesthetics that make coding fun and fulfilling.

We’re still working on a solution. My idea for the moment is to use the bugs that do get through as opportunities to improve our integration tests. We use Watir, for example to test the web site, and the controller and view code in the web site has proven too difficult to effectively unit test. I think a good mindset is to extend the lesson learned about not stopping when you get something to work to bug fixing. If a bug does get through, don’t say it’s fixed until you see a failing integration test that exposes the bug, and then see the test pass after you’ve made the fix. The fix itself then becomes, in many cases, the easiest part of bug fixing.

It’s funny how many developers think that’s overkill. I remember, way back when when I first saw the traditional waterfall model, seeing something like this:

Requirements – Design – Code – Test – Deploy

It doesn’t matter whether you’re doing waterfall or a more iterative cycle. Coding, while obviously important, doesn’t dominate that cycle. Too many developers act as if its the only part of development.

Written by Brandon Byars

June 12, 2007 at 10:44 pm

Posted in Agile

C# Execute Around Method

Kent Beck called one of the patterns in Smalltalk Best Practice Patterns “Execute Around Method.” It’s a useful pattern for removing duplication in code that requires boilerplate code to be run both before and after the code you really want to write. It’s a much lighter weight method than template methods (no subclassing), which can accomplish the same goal.

As an example, I’ve written the following boilerplate ADO.NET code countless times:

public DataTable GetTable(string query, IDictionary parameters)
{
    using (SqlConnection connection = new SqlConnection(this.connectionString))
    {
        using (SqlCommand command = new SqlCommand(query, connection))
        {
            connection.Open();
            foreach (DictionaryEntry parameter in parameters)
            {
                command.Parameters.AddWithValue(
                    parameter.Key.ToString(), parameter.Value);
            }

            SqlDataAdapter adapter = new SqlDataAdapter(command);
            using (DataSet dataset = new DataSet())
            {
                adapter.Fill(dataset);
                return dataset.Tables0;
            }
        }
    }
}

public void Exec(string query, IDictionary parameters)
{
    using (SqlConnection connection = new SqlConnection(this.connectionString))
    {
        using (SqlCommand command = new SqlCommand(query, connection))
        {
            connection.Open();
            foreach (DictionaryEntry parameter in parameters)
            {
                command.Parameters.AddWithValue(
                    parameter.Key.ToString(), parameter.Value);
            }

            command.ExecuteNonQuery();
        }
    }
}

Notice that the connection and parameter management overwhelms the actual code that each method is trying to get to. And the duplication means I have multiple places to change when I decide to do something differently. However, since the using block encloses the relevant code, a simple Extract Method refactoring is not as easy to see.

Here’s the result of applying an Execute Around Method pattern to it.

private delegate object SqlCommandDelegate(SqlCommand command);

public DataTable GetTable(string query, IDictionary parameters)
{
    return (DataTable)ExecSql(query, parameters, delegate(SqlCommand command)
    {
        SqlDataAdapter adapter = new SqlDataAdapter(command);
        using (DataSet dataset = new DataSet())
        {
            adapter.Fill(dataset);
            return dataset.Tables0;
        }
    });
}

public void Exec(string query, IDictionary parameters)
{
    ExecSql(query, parameters, delegate(SqlCommand command)
    {
        return command.ExecuteNonQuery();
    });
}

private object ExecSql(string query, IDictionary parameters,
    SqlCommandDelegate action)
{
    using (SqlConnection connection = new SqlConnection(this.onnectionString))
    {
        using (SqlCommand command = new SqlCommand(query, connection))
        {
            connection.Open();
            foreach (DictionaryEntry parameter in parameters)
            {
                command.Parameters.AddWithValue(
                    parameter.Key.ToString(), parameter.Value);
            }

            return action(command);
        }
    }
}

Much nicer, no?

Written by Brandon Byars

June 11, 2007 at 11:46 pm

Posted in .NET, Design Patterns

Tagged with

Follow

Get every new post delivered to your Inbox.