Refactoring the Disconnected LINQ to SQL Repository


I am bit blog happy tonight, but I am on a roll. In this post, I want to cover refactoring. To do this, I will be refactoring the solution I created in the last few posts (Repository Pattern in LINQ to SQL).  In our last post, I had the following routine to get the Primary Key for a LINQ to SQL object.

        private string GetPrimaryKeyName(T entity)

        {

            Type type = entity.GetType();

            foreach (PropertyInfo prop in type.GetProperties())

            {

                object[] attributes = prop.GetCustomAttributes(true);

 

                foreach (object o in attributes)

                {

                    ColumnAttribute attribute = (ColumnAttribute)o;

                    string name = o.ToString();

 

                    if (attribute.IsPrimaryKey)

                        return prop.Name;

                }

            }

 

            return null;

        }

To me, this is a lot of work for finding a primary key column. There has to be an easier way. But, before I can refactor, I need a test. Actually a suite of tests, as I want nothing to change, but here are a couple of tests that call the save method, which requires the name of a primary key.

[TestMethod()]

public void SaveNewSimStatusType()

{

    IDataContextFactory dataContextFactory = new SimStatusTypeDataContextFactory();

    IRepository<SimStatusType> target = new SqlRepository<SimStatusType>(dataContextFactory);

    string expectedName = "This is a test";

    int expectedValue = _simStatusTypeId + 1;

 

    SimStatusType entity = new SimStatusType();

    entity.SimStatusTypeId = 0;

    entity.SimStatusTypeName = expectedName;

 

    SimStatusType actual = target.Save(entity);

    isDirty = true;

 

    Assert.AreEqual(expectedName, actual.SimStatusTypeName, "name is different");

}

And

/// <summary>

///A test for Insert

///</summary>

[TestMethod()]

public void UpdateASimStatusType()

{

    IDataContextFactory dataContextFactory = new SimStatusTypeDataContextFactory();

    IRepository<SimStatusType> target = new SqlRepository<SimStatusType>(dataContextFactory);

    string expectedName = "This is a test";

    string changedName = "This is a second test";

    int simStatusTypeId;

 

    //Create entity

    SimStatusType entity = new SimStatusType();

    entity.SimStatusTypeId = 0;

    entity.SimStatusTypeName = expectedName;

    SimStatusType actual = target.Insert(entity);

    simStatusTypeId = actual.SimStatusTypeId;

 

    isDirty = true;

 

    Assert.AreEqual(expectedName, actual.SimStatusTypeName, "name is different");

 

    actual.SimStatusTypeName = changedName;

    target.Save(actual);

 

    Assert.AreEqual(changedName, actual.SimStatusTypeName, "name is different");

}

Now, I really need to refactor these tests a bit, as the warning messages tell me little and the names of the routines are not quite explicit enough for me, but that is another blog entry. Feel free to critique the tests, if you must, as I am more than happy to learn things.

When I run these routines, they come up green, which means the test is passing. I then look into how one might pull a primary key from a LINQ to SQL object. For my first refactor, I find that you can get the primary key off of the DataContext mapping. So, I change my GetPrimaryKeyName() routine to this:

private string GetPrimaryKeyName(T entity)

{

    using (System.Data.Linq.DataContext context = _dataContextFactory.Context)

    {

        MetaTable table = context.Mapping.GetTable(typeof(T));

        return table.RowType.IdentityMembers[0].Name;

    }

}

That is huge reduction in the number of lines necessary. Run tests. Still green. Okay, so I lucked out this time. But, I am now wrapping a context into a context, as the call has the context on it. Here is the save method, as it stands now.

public T Save(T entity)
{
   
using (System.Data.Linq.DataContext context = _dataContextFactory.Context)
    {
        T databaseEntity;
       
string idName = GetPrimaryKeyName(entity);
       
int id = (int)GetIdValue(entity, idName);

        //Then
        if(0 == id)
        {
           
//This is a save
            return Insert(entity);
        }
       
else
        {
            databaseEntity = context.GetTable<T>().First(s => s == entity);
            LoadDatabaseEntity(entity,
ref databaseEntity, idName);
        }

        context.SubmitChanges();
        return databaseEntity;
    }
}

This one just screams refactor, as I only need the context for the extent of the routine. The first is to call the GetPrimaryKey from outside of the context call. The other is to move the metadata pull into the save routine. Of the two, I am opting for the later until I see need for this elsewhere. While I do see a possibility, I am going to follow the rules of refactoring and only refactor as needed. Thus, a separate routine is overkill. My save routine gets change to this:

public T Save(T entity)
{
   
using (System.Data.Linq.DataContext context = _dataContextFactory.Context)
    {
        T databaseEntity;
       
string idName = _dataContextFactory.Context.Mapping.GetTable(typeof(T))
                        .RowType.IdentityMembers[0].Name;
       
//string idName = GetPrimaryKeyName(entity);
        int id = (int)GetIdValue(entity, idName);

        //Then
        if(0 == id)
        {
           
//This is a save
            context.GetTable<T>().InsertOnSubmit(entity);
            context.SubmitChanges();
           
return entity;
        }
       
else
        {
            databaseEntity = context.GetTable<T>().First(s => s == entity);
            LoadDatabaseEntity(entity,
ref databaseEntity, idName);
        }

        context.SubmitChanges();
       
return databaseEntity;
    }
}

I have now deleted the GetPrimaryKeyName() routine. Now, you might say, "but you said you would likely resurrect this code. Isn’t it better to just comment it out?" I am a bit torn on this one, but I am inclined to say no, as I can use the refactoring tools in either Visual Studio or Resharper (great product if you do not have it) to extract this method later. While this is not necessary in this routine (1 line), if I had multiple lines it would be prudent to delete now and extract later, as I would likely find some improvements in the code. I then end up copying and pasting changes, which is not a good idea when you have refactoring tools.

Just remember, the basic rule here is red … green … refactor. You then aim for green again. In my case, the change was so simple it did not break everything, but I have had cases where a refactor broke code. This is why tests are mandatory. If you are not using unit tests you should ask yourself "why am I not using them?"

Peace and Grace,
Greg

Advertisements

3 Responses to Refactoring the Disconnected LINQ to SQL Repository

  1. Jesse says:

    Greg,Do you have the full project available for download?Thanks,Jesse

  2. Gregory says:

    I will put it up somewhere and make it available for all to work with. It is still experimental (one disclaimer?).Peace and Grace,Greg

  3. Christian says:

    Hi Gregory,did you make the code available as a download?Thanks.Christian

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: