image

How do you grade how well an application is written?

There are many factors that will play a role in such an evaluation, for example (in no particular order):

  • Does the design follow object-oriented principles?
  • Does it work? (i.e. few bugs)
  • Does the code have unit tests?
  • Is the code clean (easy to read, small functions)
  • How much code duplication is there?
  • Is there valuable code comments?

Of all those qualities I will have to say readability is the most important quality, I don’t care how procedural the code is so long as the functions are small and the conditional logic written in a such a way that it is easy to follow. Don’t get me wrong I value object-orientation, the S.O.L.I.D principles and unit tests a great deal. But the majority of existing systems I have been tasked to maintain and develop new functions in seldom exhibited any those qualities.

What is usually found is a mess of procedural code where most of the code is located in the codebehind and with some common code moved to static methods on helper classes). 

What constantly surprises me is how competent and intelligent developers can create complex and functioning systems but fail to grasp the simplest of methodologies of writing readable code.

Here is some really bad code which has some of the characteristics that constantly frustrates me:

System.Web.UI.HtmlControls.HtmlInputFile objFile =
  (System.Web.UI.HtmlControls.HtmlInputFile)objControl;

try
{
  if(System.IO.Directory.Exists(strDir)==false ||
     Request["id"] != null && Request["id"] == "1" && isTpActive)
  {
    System.IO.Directory.CreateDirectory(strDir);
    
    MyApp.Business.Entities.Order order = 
      MyApp.DataAccess.Production.GetOrder((int)Request["id"]);
  }
  
}
catch(Exception ex)
{
  error = true;
  MyApp.Business.Logging.ErrorLog.Append(ex);
}

The code above is not real, I actually wrote it just show what I mean. I am having a hard time understanding how people can write code like above. Why include the namespaces in everything? This is something I constantly see and I have never understood the reason for it. Then there are the classics, like complicated conditionals that don't contain any information as what the intent of the condition is. Exception handling is also misunderstood and misused. I often find excessive capturing of exceptions, it’s like try/catch is used like guard clauses, this is very frustrating because it makes debugging and troubleshooting very painful.

Anyway, this post was not supposed to be a rant on bad code but about my realization that readability is the quality that I value most. Sure you get frustrated with procedural code that could have been so much simplified and reduced if some object orientation principles where applied but at least you don’t get a headache when trying to decipher code that is readable :)

3 comments:

Anonymous said...

Couldn't agree more man...

Tawani Anyangwe said...

Don't forget unnecessary inheritance and interface implementation.

I once worked on a huge project that had an interface for almost every since POCO and Service (Services, Repositories etc.)

Debugging was HELL. Y ou classes like this:

abstract class AggregateDomainObject : DomainObject
abstract class ProductBase : AggregateDomainObject, IProduct
abstract class Product : ProductBase
abstract class ExpiringProduct : Product
class EventProduct : ExpiringProduct

Needless to say that the interface "IProduct", the class "ProductBase" etc. were totally useless and just helped to bloat the whole app. (This is just one of the over 50 scenarios.)

Oğuz Kurumlu said...

Absolutely right!
IMHO, Readability is more important than perpormence. Because, if your code is readable. you can easily refactor your code for performance