THE “BEST PRACTICES” AND RETURN ON INVESTMENT

Author: Gediminas Geigalas

One thing that recently occupies my mind is how people tend to over-engineer their code to do things “the right way”. Development is a very creative process, involving countless hours of thinking, discussing and trying out different approaches to achieve the same thing – working software. There’s probably no two developers that would agree on how to do things best, but should there be?…

Consider the following code:

[code language=”csharp” light=”true”]
[AdminOnly]
public ActionResult Save(EditProductViewModel viewModel)
{
if (ModelState.IsValid)
{
using (var uow=repository.BeginUnitOfWork())
{
var tenant=repository.GetAll().Single(x=> x.PageId==DomainContext.Current.TenantPageId);
var product=viewModel.Id.HasValue ? repository.GetById(viewModel.Id.Value) : new Product(tenant.Id);

        product.Name=viewModel.Name;
product.Price=viewModel.Price;
product.Description=viewModel.Description;
product.FbShareDescription=viewModel.FbShareDescription;
product.LeftInStock=viewModel.LeftInStock;
if (!string.IsNullOrEmpty(viewModel.NewCategory))
{
var newCategory=new Category(DomainContext.Current.TenantId, viewModel.NewCategory);
repository.Save(newCategory);
product.Category=newCategory;
}
else
{
product.Category=repository.GetById(viewModel.CategoryId);
}
if (viewModel.PostedImage !=null && viewModel.PostedImage.ContentLength > 0)
{
product.MainImage=new ProductImage(product)
{
ImagePath=imagesService.SaveImage(viewModel.PostedImage, DomainContext.Current.TenantPageId),
ThumbnailPath=imagesService.SaveThumbnail(viewModel.PostedImage, DomainContext.Current.TenantPageId)
};
}
repository.Save(product);
uow.Commit();
}
return RedirectToAction("Edit", new{id=viewModel.Id});
}
viewModel.PostedImage=null;
viewModel.Categories=GetCategoriesList();
return View("Edit", viewModel);

}
[/code]

What do you think of this code? Is it doing too much? Probably. Does it follow best practices? Well, at least we have a Repository pattern used there, but it still looks like a mess. This code is bad. Or is it?

Udi Dahan, an exceptional speaker on SOA and also the author of NServiceBus library, likes to stress that he hates the term “best practice”. Greg Young wrote a (de)motivating blog post about a startup he participated at (http://goodenoughsoftware.net/2013/03/06/startups-and-tdd/). His point proves that going quick and dirty is a valid way. Even Uncle Bob has been caught saying that he doesn’t always do Test Driven Development – practice that he so actively proposes. What drives those known specialists towards those conclusions?

It’s all about the context and return on investment. Let’s get back to the code example. If I found something like this during a code review, I’d probably suggest some ideas to improve it. This code might need refactoring, because the logic leaks out into controller action. What can we do about it? Maybe introduce an application layer, write some tests, extract multiple smaller methods.

But hey, what if I tell you that I wrote this overnight to validate a startup idea? Things change drastically. If I would do this again, I’d even throw the Repository pattern out – it was a nuisance. See, when no one is paying you for your work, you tend to think of ways to do things most effectively – to try out the product before making it more stable and pretty. Did I write automated tests? No. I saw no value in them: I was working alone, there wasn’t too much code and I had no idea if I’ll ever release the code.

That’s what return on investment is all about. Don’t pay the price if what you get in return is of less value.

And it’s not only related to startups or throw-away code. Some of the choices that I’ve made in my professional projects recently and that go against the “best practices”:

• Using NHibernate ORM directly, without Repository abstractions – I’m almost 100% sure that no other ORM will be able to do what I need with a legacy database in near future. I’m also 110% sure that there will be no transition to NoSql solutions.
• Keeping ORM mappings and Domain Model in the same project – again, not likely that there will ever be a need to change ORMs. – Not introducing a “service layer” (web services, rest services, etc.) – there’s only one single entry point to the domain logic, why should I pay the price of complexity? And no, potentially being able to add another UI in the future does not seem like a realistic argument.
• Writing more integration tests that unit tests – we’re using an ever changing legacy DB, so proper integration is our main concern. Running things in memory does not prove anything.
• Always try to find the things are least likely to change – they should be the main pillars of your architectural and code design decisions. If there is no need to complicate things, then go with the simple implementation, since all the complexity costs you your time and clients money. If things change – adapt quickly. One of my favorite quotes is “Fool me once – shame on You! Fool me twice – shame on me” found in “Agile Principles, Patterns, and Practices in C#” book by Uncle Bob. Translating it to software development means that your code design should react to changes in requirements as they occur. And they inevitably will.

In conclusion, it should be understood that there is no best solution to any problem in software – “good enough”, in the appropriate context, should be the goal. And yes, it is very subjective. If it wasn’t, we’d all be using the same tools and processes day after day and die from boredom.