in

Dot Net Mafia

Group site for developer blogs dealing with (usually) .NET, SharePoint 2013, SharePoint 2010, Office 365, SharePoint Online, and other Microsoft products, as well as some discussion of general programming related concepts.

Cory Robinson's Blog

Code Readability => Code Maintainability

As developers the readability of our code is what determines how easily either we ourselves or other developers can maintain our code.  Sometimes we can accomplish the same functionality in fewer lines of code, or using some fancy tricks, but this can be bad for maintainability.  Sure, there are some situations where some of the code is repeated unnecessarily, or may not be needed at all, and can be made both simpler and easier to read by refactoring it.  But rewriting readable code in fewer lines such that anyone who looks at it will spend considerable time understanding what it is doing is no good.

 

Extensive use of the ternary conditional operator is one sign your code might not be as readable as it should be.  For example, the following code works correctly and in my opinion is pretty readable as to what its intended functionality is supposed to be:

 

if (!productStartAdded)

{

// Add with an OR unless worklist criteria count is 0

if (filterCount == 0)

{

worklistCriteria.AddFilterField(WCLogical.And, WCField.ProcessStatus, WCCompare.Equal, k2ProcessInstanceStatuses[j]);

filterStringBuilder.Append(" && ProcessStatus == " + k2ProcessInstanceStatuses[j].ToString());

}

else

{

worklistCriteria.AddFilterField(WCLogical.Or, WCField.ProcessStatus, WCCompare.Equal, k2ProcessInstanceStatuses[j]);

filterStringBuilder.Append(" || ProcessStatus == " + k2ProcessInstanceStatuses[j].ToString());

}

 

productStartAdded = true;

}

else

{

worklistCriteria.AddFilterField(WCLogical.And, WCField.ProcessStatus, WCCompare.Equal, k2ProcessInstanceStatuses[j]);

filterStringBuilder.Append(" && ProcessStatus == " + k2ProcessInstanceStatuses[j].ToString());

}

 

Note that the example code above is using the K2ROM API for K2.NET.  If there were an overload of the AddFilterField method that took just the And or Or, the code would be simplifyable and retain its readability but unfortunately there is no such overload.

 

But say we decide that we can do some fancy maneuvering and rewrite these 19 lines of code in just three lines!  Wow, that would be cool, right?  Take a look at those three lines, which have the same functionality as the original 19:

 

worklistCriteria.AddFilterField(productStartAdded && filterCount != 0 ? WCLogical.And : WCLogical.Or, WCField.ProcessStatus, WCCompare.Equal, k2ProcessInstanceStatuses[j]);

 

filterStringBuilder.Append((productStartAdded && filterCount != 0 ? " && " : " || ") + " ProcessStatus == " + k2ProcessInstanceStatuses[j].ToString());

 

productStartAdded = true;

 

That is downright amazing.  The compiler is going to thank us for that, right?  Or is it?  Well, the original version results in 91 lines of IL code, while the rewritten version is just 49 lines of IL.  That is certainly not the roughly 6:1 ratio that we saw in the C# code, nor is it even 2:1 in IL.

 

One might expect about a 46% performance boost based on those IL line counts.  To test this, I actually profiled the code in the ANTS Profiler, executing each method 1000000 times. The results showed no performance boost at all, rather a slight decrease in performance:

 

Original: 12.2521 seconds                    Rewritten: 12.8333 seconds

 

Amazing!  Or is it?  When you look at what the code is actually doing, the original version is going to have 3-5 of its lines executed depending on the value of productStartAdded and filterCount.  The rewritten version will always have all three of its lines executed.  So the performance metrics that we obtained now make sense.

 

So now that we have painstakenly shown that, even though you think it will impress your friends, rewriting the 19 lines of code into three lines doesn’t help performance a bit.  What should we then conclude about this result?

 

The conclusion that I take away from this is that, since the performance is basically the same in both cases, I would rather see the original 19 lines of code that I can read and determine easily what the purpose of the code is, versus seeing the rewritten three lines of code and sitting there scratching my head for an hour trying to decipher these cryptic statements.  So would your fellow developers who are going to maintain your code.

Read the complete post at http://www.dotnetmafia.com/Home/tabid/37/EntryID/61/Default.aspx

2015 dotnetmafia.
Powered by Community Server (Non-Commercial Edition), by Telligent Systems