exampleCode != productionCode

Take a look at this little piece of code. It looks pretty innocuous, like it was taken straight from "Teach Yourself ASP.NET in 21 Days". Pull a list of Trips out of the database for a given user, and bind it to a select list. Nothing fancy. Teaches you a little bit about ADO.NET and databinding all in one place.

Figure 1., Book Sample

public class ExampleCode : System.Web.UI.Page
{
    protected HtmlSelect selTripID;

    private void Page_Load(object sender, System.EventArgs e)
    {
        if (!IsPostBack)
        {
            DataSet ds = new DataSet();
            SqlConnection connection = new SqlConnection(
                "server=OurProductionServer;database=Payroll;
                UID=jimmy;PWD=j1mmy;");
            SqlDataAdapter adapter = new SqlDataAdapter(
                "select * from Trip where UserID=" 
                + Request["UserID"], connection);
            adapter.Fill(ds);

            selTripID.DataSource = ds;
            selTripID.DataTextField = "TripName";
            selTripID.DataValueField = "TripID";
            selTripID.DataBind();
        }
    }
}

Imagine my surprise, however, when I walked in to a small software shop recently and found a whole project written with code like the above. What were these guys thinking? Are they seriously relying on this fragile, unmaintainable mess in a real software product?

And then it dawned on me. Maybe nobody had ever told them that the little examples in the book are just that: Little Examples. For teaching purposes. Never intended for use in the real world. Come to think of it, it doesn't even tell you that in the book. It aught to be in block capitals across the cover of the book:

WARNING: DO NOT PASTE THE SAMPLES FROM THIS BOOK DIRECTLY INTO PRODUCTION SOFTWARE!!!

Somehow, it seems that this message never got through to a substantial portion of the software industry. Every time I see a "Senior Developer" writing ad-hoc SQL or referencing a hashtable with a string I just want to cry.

So what do we do about it? I guess we try to get the message out. Here is some code I copied out of the Blogabond source that is functionally equivalent to the above:

Figure 2., Production Sample

public class ProductionCode : System.Web.UI.Page
{
    protected HtmlSelect selTripID;
    private int _userID;

    private void Page_Load(object sender, System.EventArgs e)
    {
        if (!IsPostBack)
        {
            _userID = StringConvert.ToInt32(
                Request[User.Columns.UserID], 0);
            if (_userID != 0)
            {
                PopulateTripList();
            }
            else
            {
                // bail gracefully...
            }
        }
    }

    private void PopulateTripList()
    {
        selTripID.DataSource = Trip.GetByUserID(_userID);
        selTripID.DataTextField = Trip.Columns.TripName;
        selTripID.DataValueField = Trip.Columns.TripID;
        selTripID.DataBind();
    }
}

Short and to the point. And obviously only the tip of the iceberg. This bit of code goes deep, but we can learn a few things just by looking at it:

1. We're using a Data Layer of some sort. Somewhere in the back end lives a class that is wrapping a stored procedure for me. There's a ConnectionFactory back there someplace that knows to hand me a connection to the Production database because of a server setting.

   The code that's handing me the DataSet I'm binding to can be reused by any page in the project, so I know that little select statement is only living in a single place. In fact, everything that touches that table is sitting in a single class back there. So if something changes in the schema I won't have to go hunting through the client code to fix it.

   Best of all, because we've separated the database code out into its own place, we can drop all the boilerplate CRUD into CodeSmith templates and let it auto-generate itself from the database schema. In the case of Blogabond, I can flip a switch and watch as 50k lines of boilerplate C# and SQL in our backend gets blown away and recreated in about 30 seconds. And since it's also generating all the boilerplate Unit Tests for all that code, we can be sure that the datalayer and the database line up correctly. So if junior dev Jimmy comes by and unchecks a not-null constraint in Enterprise manager, we'll see the continuous build fail on an integrity check a few minutes later.

2. We're using Enums instead of inline strings to reference our column names. This may not sound like a big deal, but it buys us a couple things. First, we've abstracted out the concept of the Typo. There is simply no way we can misspell "TripID", because the compiler will catch it for us. It will sit there underlined in red if we even try. We don't even have to remember what columns we have available, since Intellisense will tell us when we hit ctrl+space.

   Second, and bigger, is that we buy the freedom to monkey with our tables and never worry that we're causing runtime errors someplace. I can rename the column "TripName" to "BlogName" if I want, re-generate the database wrappers, and watch as the project fails to compile until I fix the references. That is some powerful stuff. And what happens if I forget to re-generate the wrappers for that table? The continuous build will fail in about 5 minutes, since the unit tests that check to make sure the wrappers match the schema will fail.

Where do we go from here?

Copy and paste code reuse is bad. Everybody knows that. Ad-hoc SQL is bad. Everybody knows that. Inline strings are bad. Everybody knows that.

At least that's what I thought. But you know what? They don’t. And they should. And it's our job to tell them.

Discuss on hacker news