Coding Efficiency

Today a new trainee was assigned to me. He just completed his in-firm training in another company and applied for a job.

In order to find out what he knows, I thought it would be a good idea to go through the code of some tools.

We began with a tool that prints invoices. It has two Forms and no architecture at all. Although the code was a mess, he could easily tell how it worked.

After that, we looked at a tool that queried some data, applied some validation to it and if necessary corrected it. This tool used the command pattern for validation / correction and loaded those commands via reflection (about 50 in total). It was way harder for him to tell what was going on. He asked why anyone would do things this complicated and suggested to use a switch block instead.

The third tool was a little more complicated, the UI used the passive view pattern, persistence was dealt with by NHibernate, StructureMap wired it all up and a test suite covered most parts. I wrote it some time ago but could still easily navigate through it as it was well structured but he was totally overwhelmed and unable to say anything other than “it is too complicated”.

Using these good practices has advantages but these advantages are not free.

There are way more developers like my trainee that never heard about good practices and the like than those who heard about these practices let alone applying them. These developers usually just want to get the job done and mostly they do so, on time.

If you work in a very small IT department that only deals with in house stuff, should you use patterns, unit testing and other good practices or should you just do it “the good old way”?

If you do it “the good old way” maintenance costs increase, time to bug fix / feature are less predictable yet everyone should be able work with it.

If you use good practices, maintaining your applications either can’t be done by every “developer from the street” or requires high upfront learning effort. On the other side, after this initial learning phase is over, the maintenance costs should decrease and time to bug fix / feature should be more predictable.

As I was looking for another open source project for my refactoring series, I saw both, very good and very bad code.

As many projects, this one just wanted to implement three tiers, with the data and the business layer having their own data containers. Although they are equal by content, they are different classes.

I really don’t understand why anyone would do such a thing:

// In some method that needs an "Category" object
Category category = DBMapping(dbItem);

// Somewhere else, revealing what the dbItem is....
private static Category DBMapping(DBCategory dbItem)
{
    if (dbItem == null)
        return null;

    Category item = new Category();
    item.CategoryID = dbItem.CategoryID;
    item.Name = dbItem.Name;
    item.Description = dbItem.Description;
    item.TemplateID = dbItem.TemplateID;
    item.MetaKeywords = dbItem.MetaKeywords;
    item.MetaDescription = dbItem.MetaDescription;
    item.MetaTitle = dbItem.MetaTitle;
    item.SEName = dbItem.SEName;
    item.ParentCategoryID = dbItem.ParentCategoryID;
    item.PictureID = dbItem.PictureID;
    item.PageSize = dbItem.PageSize;
    item.PriceRanges = dbItem.PriceRanges;
    item.Published = dbItem.Published;
    item.Deleted = dbItem.Deleted;
    item.DisplayOrder = dbItem.DisplayOrder;
    item.CreatedOn = dbItem.CreatedOn;
    item.UpdatedOn = dbItem.UpdatedOn;

    return item;
}

Doing so provides nothing, you just violate DRY and thereby create your self some great amount of repetive work. Further, human errors like forgetting a property will arise.

Another thing I encountered rather often is the following:

private IList<Category> GetAllCategories()
{
    return DataLayer.CategoryManager.GetAllCategories();
}

The so called business layer is nothing more than a very thin proxy to the data layer. If you used interfaces for decoupling in both, the business and the data layer, you have to change 3 places in exactly the same way, doesn’t that feel just wrong?

If all you want to do is to allow different data stores, then just do that. There is absolutely no reason to duplicate your data containers in such a case.

You have at least three ways:

Let the different data layer implementations depend on the business layer. Define the data containers and the interfaces to retrieve those in the business layer and let your IoC container do the rest.

Create a separate assembly that contains the data containers. You can still let the business layer depend on the data layer, yet you don’t have to duplicate anything.

Use an ORM, like NHibernate.

If you ever created a project with just 2 parts, usually named ui/gui/presentation and core/logic/service, it’s actually the same. The only difference is that you implement the interfaces by which you retrieve your entities / business object, or however you want to call them, in a separate assembly. Done. Data stores of all kind can be implemented.

By doing so, you can unit test, without having to hit your real data store. Further, you can switch databases, just load a different data layer implementation and its done.