Working with legacy code is always one of my favorite things to do.  I believe it helps me write more maintainable code, and the little mysteries are always quite engaging.  The most recent example that I feel compelled to share is related to the setting of a database value based on the selection made in a radio button group.  In this particular instance, a set of custom localization routines help cloud the issue by abstracting the text from the code, but the central fault is more universal.  There was an undocumented naming convention that was initially followed within the database, but the constraints on columns were not always put in place to enforce the convention and the code itself was not designed to assist in any way.

On to specifics (altered to insure confidentiality). 

• Initially, in the UI there are three radio buttons that correspond to an integer column in the database (color).  The value is set on a public property of an object that corresponds to the table (widgets) in the database.  There is a corresponding dimension table (widgetColorCode) in the database, but there is a missing foreign key constraint and the naming convention doesn’t make it easy to track down the dimension domain.
• Time passes.
• A new UI is built for a new company that only manufactures red Widgets.  The combo boxes are removed, and the value is set upon submission to the constant 1. 
• A derivative of the new UI is commissioned that is for a company only supplying blue Widgets.   There is nothing in the code to help the programmer find what value this property should have. 

So the code for the Widget class looks something like this:

    public class Widget
    {
        public int Color { get; set; }
    }

I am a firm believer in having enumerations corresponding to all dimensions in a project.  There is a performance hit for converting an enumeration to an integer, but it is far outweighed by the increased maintainability of the code.  By explicitly setting the values for the enumeration and adding an XML comment relating it to the dimension table, you can greatly increase the maintainability of the code. When starting from scratch on a project, I will typically have something along the lines of:

    ///

    /// corresponds to the Widget fact table
    /// 

    public class Widget
    {
        public WidgetColor Color { get; set; }
    }

    ///

    /// corresponds to the WidgetColorCode dimension table
    /// 

    public enum WidgetColor
    {
        Red = 1,
        White = 2,
        Blue = 3
    }

But, that is a personal preference, and refactoring the entire code base at this point can’t be justified.  What can be done quickly to lessen the impact of this when further maintenance is required?  Visual Studio comes to the rescue!  By simply adding an XML comment to the property, we can define the dimension so that hovering over the property will display the dimension information (Please excuse the missing pointer in the image, trust me, I am hovering over the Color property.):

    ///

    /// corresponds to the Widget fact table
    /// 

    public class Widget
    {
        ///

        /// corresponds to the WidgetColorCode dimension table
        /// 1: Red, 2: White, 3: Blue
        /// 

        public int Color { get; set; }
    }

C# · DotNet · enumerations · Example · IntellliSense · Legacy code · Maintainability · Visual Studio

I have been working with Telerik controls for a little over two months (the ASP.NET AJAX suite) and have been continuously impressed with the quality and power they provide.  I am working on a project that requires the upload of several different file types which need to be displayed in a grid once uploaded.  The cleanest solution I could think of was to create a User Control to handle the uploads and embed it within the grid through the EditFormSettings FormTemplate.  So first, I created a simple upload User Control as follows:

<%@ Control Language="C#" AutoEventWireup="true" CodeBehind="UploadControl.ascx.cs"
    Inherits="WebApplication1.UserControls.UploadControl" %>
    
    
    
    
    
    Submit

with the code behind set up to store the uploaded files in the database using LINQ:

        protected void btnSubmit_Click(object sender, EventArgs e)
        {
            LinerModelDataContext dataContext = new LinerModelDataContext();
            foreach (UploadedFile file in RadUpload1.UploadedFiles)
            {
                byte[] bytes = new byte[file.ContentLength];
                file.InputStream.Read(bytes, 0, file.ContentLength);
                Attachment attachment = new Attachment();
                attachment.FileName = file.GetName();
                attachment.BinaryAttachment = bytes;
                attachment.CustomerID = this.CustomerID;
                dataContext.Attachments.InsertOnSubmit(attachment);
            }
            dataContext.SubmitChanges();
        }

I then just added the EditFormSettings section to RadGrid1 and I was able to upload files:

But my grid wasn’t refreshing and I wasn’t getting any notification that the upload was complete. So, to provide the user experience I wanted, I exposed an event to the grid from the upload control by modifying the code behind of the UserControl. I added a delegate, and exposed the event as follows:

    public delegate void UploadHandler();

    public partial class UploadControl : System.Web.UI.UserControl
    {
...
        public event UploadHandler Upload;

        protected void btnSubmit_Click(object sender, EventArgs e)
        {
            LinerModelDataContext dataContext = new LinerModelDataContext();
            foreach (UploadedFile file in RadUpload1.UploadedFiles)
            {
                byte[] bytes = new byte[file.ContentLength];
                file.InputStream.Read(bytes, 0, file.ContentLength);
                Attachment attachment = new Attachment();
                attachment.FileName = file.GetName();
                attachment.Attachment1 = bytes;
                attachment.CustomerID = this.CustomerID;
                dataContext.Attachments.InsertOnSubmit(attachment);
            }
            dataContext.SubmitChanges(ConflictMode.ContinueOnConflict);
            if (Upload != null)
            {
                Upload.Invoke();
            }
        }

and changing the FormTemplate to be:

(please note that the syntax highlighter I am using mangled the code by changing OnUpload to be onupload in the above source) and adding the handler to the main page:

        protected void UploadControl1_Upload()
        {
            RadGrid1.MasterTableView.IsItemInserted = false;
            RadGrid1.Rebind();
        }

Event · Example · Telerik · UserControl

I love working with legacy code, really I do.  I find the effort of trying to put myself mindset of the people who originally coded the applications enjoyable.  It is somewhat satisfying to be able to get enough of a feel for the code to get a feel for what they were attempting to do as well as what got done.  In most cases, it is quite easy to see which libraries and sections were implemented first, and which were either rushed to completion or tacked on as an afterthought.  It often makes me spend the extra 15 minutes to fully implement something rather than simply patching over a flaw and planning on implementing the ‘real code’ at some mythic future date.  Coming in cold to a project, doing a search for TODO is a good start, but I have also found a lot of cases where that isn’t good enough; so, I have begun setting Visual Studio (VS) to break on any thrown exception.  If you aren’t familiar with how to change the way VS handles exceptions in debug mode, here is where you can make the changes:

Doing this helps me track down gems like this (from actual code, I made minor changes to keep from exposing client specific information):

            //TEMPORARY - make sure the ID contains at least one alpha
            try
            {
                int i = int.Parse(newID);
                newID = newID.Remove(4, 1).Insert(2, "X");
            }
            catch { }

To be fair to the coder, TryParse has only been in the framework since 2.0, and this code could well have have begun life as a 1.1 application, but the TEMPORARY in the comment was obviously overly optimistic.  My solution was to replace the above with:

            int i;
            //make sure the ID contains at least one alpha
            if (int.TryParse(newID, out i))
            {
                newID = newID.Remove(4, 1).Insert(2, "X");
            }

Exceptions · Legacy · Visual Studio

Tightly-coupled, loosely bound. This has been my mantra for enterprise software development for the past 7 or so years. I have always been in favor of compile time over run time errors at (nearly) any cost. I don’t feel that I am dogmatic about it, I understand the need for flexibility in internal interfaces and early in a projects life cycle. But, at the point you have down stream systems that depend on your application to perform, the rules change. You are reading this entry using an application that has a loosely coupled, forgiving interface with the content it receives for rendering (see Joel’s discussion of inherent folly of this choice http://www.joelonsoftware.com/items/2008/03/17.html), so large systems don’t require the level of synchronization I prefer to function. I believe, however, they require it to function well.

Apparently, I am not alone in my belief. The .NET 4.0 framework now has an mscorlib level component that helps enforce the coupling at compile time. The System.Diagnostics.Contracts namespace contains a suite of static classes that, coupled with a post-processing code re-writer, provides static and run time implementations of preconditions, post-conditions, and invariants. In this post I am going to start with some simple examples of how the syntax works. In future posts, I hope to delve into the implications of this system as well as how our best practices need to be updated to take advantage of the power of these structures.

The first thing you need to do is to install the new contract tool set as outlined in http://blogs.msdn.com/bclteam/archive/2010/01/26/i-just-installed-visual-studio-2010-now-how-do-i-get-code-contracts-melitta-andersen.aspx. Please note that unless you are using Premium or Ultimate, you will not get Static evaluation, only run time.

You then turn static checking on in the settings in Project Properties > Code Contracts and select “Perform Static Contract Checking” so it looks like this:

Through static checking, we should see compile time errors. Unfortunately, the combination of VS 2010 RC I am using along with the Release 1.2.30312.0 of the Code Contracts for .NET doesn’t have the static version working properly. So for this example we are going to have to look at run time errors. That is set on the same Project Property page:

Lets start with a simple sample program with some properties.

    class Program
    {
        public string Name { get; set; }
        public int Age { get; set; }

        static void Main(string[] args)
        {
            Program program = new Program();
            program.Age = -7;
            program.Name = "jon";
        }
    }

Within the program above, we can see a common pattern for properties. If we look at the underlying IL of one of the setter methods, we see what we should expect:

.method public hidebysig specialname instance void
        set_Age(int32 'value') cil managed
{
  .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 )
  // Code size       8 (0x8)
  .maxstack  8
  IL_0000:  ldarg.0
  IL_0001:  ldarg.1
  IL_0002:  stfld      int32 Contracts.Program::'k__BackingField'
  IL_0007:  ret
} // end of method Program::set_Age

After modifying the code to include the required using statement:

using System.Diagnostics.Contracts;

and updating the property to have the following contract statement:

        public int Age
        {
            get { return _age; }
            set
            {
                Contract.Requires(value >= 0);
                _age = value;
            }
        }

When a command line program is run with a bad argument, the following exception is thrown:

Unhandled Exception: System.Diagnostics.Contracts.__ContractsRuntime+ContractException: Precondition failed: value >= 0
   at System.Diagnostics.Contracts.__ContractsRuntime.TriggerFailure(ContractFailureKind kind, String message, String userMessage, String conditionText, Exception inner) in :line 0
   at System.Diagnostics.Contracts.__ContractsRuntime.ReportFailure(ContractFailureKind kind, String message, String conditionText, Exception inner) in :line 0
   at System.Diagnostics.Contracts.__ContractsRuntime.Requires(Boolean condition, String message, String conditionText) in :line 0
   at Contracts3.Program.set_Age(Int32 value) in c:\Contracts\Program.cs:line 19
   at Contracts.Program.Main(String[] args) in c:\Contracts\Program.cs:line 33

and looking at the IL I find where the rewrite has added wrappers to handle the pre-conditions:

.method public hidebysig specialname instance void
        set_Age(int32 'value') cil managed
{
  // Code size       74 (0x4a)
  .maxstack  3
  IL_0000:  ldsfld     int32 System.Diagnostics.Contracts.__ContractsRuntime::insideContractEvaluation
  IL_0005:  ldc.i4.4
  IL_0006:  bgt        IL_003c
  .try
  {
    IL_000b:  ldsfld     int32 System.Diagnostics.Contracts.__ContractsRuntime::insideContractEvaluation
    IL_0010:  ldc.i4.1
    IL_0011:  add
    IL_0012:  stsfld     int32 System.Diagnostics.Contracts.__ContractsRuntime::insideContractEvaluation
    IL_0017:  ldarg.1
    IL_0018:  ldc.i4.0
    IL_0019:  clt
    IL_001b:  ldc.i4.0
    IL_001c:  ceq
    IL_001e:  ldnull
    IL_001f:  ldstr      "value >= 0"
    IL_0024:  call       void System.Diagnostics.Contracts.__ContractsRuntime::Requires(bool,
                                                                          string,
                                                                          string)
    IL_0029:  nop
    IL_002a:  leave      IL_003c
  }  // end .try
  finally
  {
    IL_002f:  ldsfld     int32 System.Diagnostics.Contracts.__ContractsRuntime::insideContractEvaluation
    IL_0034:  ldc.i4.1
    IL_0035:  sub
    IL_0036:  stsfld     int32 System.Diagnostics.Contracts.__ContractsRuntime::insideContractEvaluation
    IL_003b:  endfinally
  }  // end handler
  IL_003c:  nop
  IL_003d:  ldarg.0
  IL_003e:  ldarg.1
  IL_003f:  stfld      int32 Contracts3.Program::_age
  IL_0044:  br         IL_0049
  IL_0049:  ret
} // end of method Program::set_Age

The re-writing is being done by ccrewrite, and can be conditionally compiled in. There is also a structure set up to allow runtime code to catch the contract exception to be handled internally.
links: http://msdn.microsoft.com/en-us/library/system.diagnostics.contracts(v=VS.100).aspx

No tags

public string Name { get; set; }

syntactic sugar, it still isn’t as clean or concise as Ruby:

class Person
   attr_accessor :first_name, :last_name
end

or what he envisioned for Java:

//context of another
class Person p = new Person();
p.firstName := "John";

where the getter and setter are automatically appended to the Person object by the compiler.

Well, .NET 4.0 still isn’t quite as cool and concise, but it is getting closer with the introduction of the ExpandoObject. By adding the following using statement:

using System.Dynamic;

you now get the ability to write code like:

dynamic p = new ExpandoObject();
p.firstName = "John";

Oh, and did I mention that you get IntelliSense with it as well?

You still get IntelliSense with ExpandoObject dynamic properties.

When using ExpandoObject(s) with .NET 4.0 and C#, you get IntelliSense for all of the dynamically generated fields.

link: http://msdn.microsoft.com/en-us/library/system.dynamic.expandoobject(v=VS.100).aspx

No tags