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

May 2007 - Posts

  • 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.

  • Implementing a Service Locator

    One of the problems with using a factory is that typically you have to change your factory when you add new classes of objects that can be created. I decided that this was not good enough, so instead used a configuration file to list out the types that the factory can create. This is commonly referred to as a Service Locator.  This post describes how to implement one.

    This uses the System.Activator class to create objects of the specified type at runtime. The configuration file just uses AppSettings to store key/value pairs of type names and fully qualified (with namespace) type names. As more classes are added that need to be created by this factory, all that is required is to add a new configuration file key/value pair for each new class.

    Since this is done at runtime, I would assume there is a performance hit doing it this way. But it's yet another tradeoff between extensibility and performance. What is right for your specific application will of course depend on the level of performance that you need.

    This example will use an abstract Report class and three concrete classes that derive from Report.

    First of all, we have the abstract class which all classes we will instantiate derive from:

    abstract class Report

    {

    protected ReportViewer reportViewer;

    protected ReportCreationParameters creationParameters;

           

    protected Report(ReportCreationParameters parameters)

    {

    creationParameters = parameters;           

    }

     

    protected abstract void DisplayReport (ReportDisplayParameters parameters);

    }

     

    Now an example of a derived class:

     

    class ReportTypeA : Report

    {

    public ReportTypeA(ReportCreationParameters parameters) : base(parameters)

    {

    // Create report based on the parameters passed in

    }

     

    protected override void DisplayReport(ReportDisplayParameters parameters)

    {

    // Display method specific to report type A

    }

    }

     

    The classes referred to above are very simple, and will use Reporting Services types and settings eventually.  The ReportCreationParameters class contains settings that will be used to create the report, while the ReportDisplayParameters class contains settings that will be used to display the report.

     

    class ReportViewer { }

     

    class ReportCreationParameters

    {

    private string parameter1;

    private string parameter2;

     

    public string Parameter1

    {

    get { return parameter1; }

    set { parameter1 = value; }

    }

     

    public string Parameter2

    {

    get { return parameter2; }

    set { parameter2 = value; }

    }

    }

     

    class ReportDisplayParameters { }

     

    Here is the AppSettings section of the configuration file:

     

      <appSettings>

        <add key="ReportTypeA" value="DotNetMafia.ReportTypeA"/>

        <add key="ReportTypeB" value="DotNetMafia.ReportTypeB"/>

        <add key="ReportTypeC" value="DotNetMafia.ReportTypeC"/>

      appSettings>

     

    Now the interesting part, the factory, which uses System.Activator and AppSettings to instantiate the types specified in the configuration file. I used the CreateInstance overload that takes the class type and an array of objects, which are the parameters to the constructor (ReportCreationParameters in this example):

     

    static class ReportFactory

    {

    public static Report CreateReport(string reportType, ReportCreationParameters parameters)

    {

    string reportClassType = ConfigurationManager.AppSettings[reportType];

    return (Report)Activator.CreateInstance(Type.GetType(reportClassType), new object[] { parameters });

    }

    }

     

    So then from the main program I use this code to instantiate whatever class I need:

     

    ReportCreationParameters parameters = new ReportCreationParameters();

    parameters.Parameter1 = "TypeAParameter1";

    parameters.Parameter2 = "TypeAParameter2";

    ReportFactory.CreateReport("ReportTypeA", parameters);

     

    So there you have it.  This is nothing profound or even original, but in my opinion it is a good way to use factories with extensibility in mind.

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