One of my personal pet peeves in development is the use of “Magic Numbers”in code.  I’m even so retentive to include string literals in my disdain of unmanageable code.  This especially becomes prevalent in data access code.

My nightmare

Handing me a piece of code that looks like this:

public class Foo
{
    public string Bar { get; set; }
    public string Id { get; set; } 
    public string Title { get; set; } 
    public string Description { get; set; } 
    public string CreatedOn { get; set; } 
    public string CreatedBy { get; set; }
}

public class FooManager
{
    public Foo GetFooById(string Id)
    {
        var retFoo = new Foo(); 
        var param = new SqlParameter("Foo_Id", SqlDbType.VarChar, 25); 
        param.Value = Id; 
        
        using (var conn = new SqlConnection("Data Source=FooServer;Initial Catalog=FooDb;User Id=FooUser;Password=FooPassword"))
        {
            using (var cmd = new SqlCommand("FOO_FETCH_BY_ID", conn))
            {
                cmd.CommandType = CommandType.StoredProcedure; 
                cmd.Parameters.Add(param); 
                var rdr = cmd.ExecuteReader(CommandBehavior.CloseConnection); 
                if (rdr != null)
                {
                    rdr.Read(); 
                    retFoo.Bar = rdr["Foo_Bar"].ToString(); 
                    retFoo.Id = rdr["Foo_Id"].ToString(); 
                    retFoo.Description = rdr["Foo_Description"].ToString(); 
                    retFoo.Title = rdr["Foo_Title"].ToString(); 
                    retFoo.CreatedOn = rdr["Foo_CreatedOn"].ToString(); 
                    retFoo.CreatedBy = rdr["Foo_CreatedBy"].ToString();
                }
            }
        } return retFoo;
    }
}

is one of the quickest ways to fail a code review.

The code is littered with string literals.. and a blasted magic number!  (I know that the connection string is going overboard in this example.. but it’s there because I’ve seen it before)

Let’s do something a bit different:

public class Foo
{
    public struct DBFields
    {
        public const string Bar = "Foo_Bar";
        public const string Id = "Foo_Id";
        public const string Title = "Foo_Title";
        public const string Description = "Foo_Description";
        public const string CreatedOn = "Foo_CreatedOn";
        public const string CreatedBy = "Foo_CreatedBy";
    }

    public struct DBLengths
    {
        public const int Bar = 35;
        public const int Id = 25;
        public const int Title = 65;
        public const int Description = 500;
        public const int CreatedOn = 12;
        public const int CreatedBy = 20;
    }

    public string Bar { get; set; }
    public string Id { get; set; }
    public string Title { get; set; }
    public string Description { get; set; }
    public string CreatedOn { get; set; }
    public string CreatedBy { get; set; }
}

public class FooManager
{
    private const string DBSTRING = "Data Source=FooServer;Initial Catalog=FooDb;User Id=FooUser;Password=FooPassword";

    private struct Procs
    {
        public const string FooById = "FOO_FETCH_BY_ID";
        public const string FooByBar = "FOO_FETCH_BY_BAR";
        public const string AllFooByCreator = "FOO_FETCH_ALL_BY_CREATOR";
    }

    public Foo GetFooById(string Id)
    {
        var retFoo = new Foo();
        var param = new SqlParameter(Foo.DBFields.Id, SqlDbType.VarChar, Foo.DBLengths.Id);
        param.Value = Id;

        using (var conn = new SqlConnection(DBSTRING))
        {
            using (var cmd = new SqlCommand(Procs.FooById, conn))
            {
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.Add(param);
                var rdr = cmd.ExecuteReader(CommandBehavior.CloseConnection);
                if (rdr != null)
                {
                    rdr.Read();

                    retFoo.Bar = rdr[Foo.DBLengths.Bar].ToString();
                    retFoo.Id = rdr[Foo.DBFields.Id].ToString();
                    retFoo.Description = rdr[Foo.DBFields.Description].ToString();
                    retFoo.Title = rdr[Foo.DBFields.Title].ToString();
                    retFoo.CreatedOn = rdr[Foo.DBFields.CreatedOn].ToString();
                    retFoo.CreatedBy = rdr[Foo.DBFields.CreatedBy].ToString();
                }
            }
        }
        return retFoo;
    }
}

Wait.. What?!?

Alright, here’s what we did.. First we created a struct in the Foo object called DBFields.  This represents of the Foo Fields as they are so named in the DB.  Notice that it’s a struct of string constants.

The next thing we did was to create a struct of DBLengths in the Foo object.  This represents the length definition of all the fields in the Foo object as they correlate to the DB Definition.  Notice that it’s a struct of int constants.

In the FooManager, we created another struct called Procs.  This represents the name of each stored procedure that the FooManager users as it interacts with the DB in reference to the Foo object.

Most importantly, in the GetFooById() method.. there are no “Magic Numbers”  All that information is handled in one spot. 

You want to expand the Foo.Description field to 1000 instead of 500?  Modify the Foo.DBLengths.Description and the change will occure in every piece of code that uses the description length.  The Foo.Id property isn’t long enough?  Expand it and modify it in one place.

The stored procedure Foo_FETCH_By_ID is being re-written and the new name is FOO_FETCH_BY_ID2 and now you want to reference it by the new name..  Instead of search and replace, just modify the FooManager.Procs.GetFooById field and you’re good to go.

So what are you saying?

Magic Numbers are Bad.  Don’t use them.