Broloco

In my continuing adventures with legacy code I've inherited a code base that initially had over 500 compiler warnings. Now many of these warnings were pretty straightforward to fix but some of them are proving a little more interesting. My current personal favourite is CS0252.

I've got around half a dozen places where it's really unclear as to exactly what the original programmer was intending to do. Was he/she really meaning to do a reference comparison? Unlikely. So now I've got an interesting predicament. Do I change the code to remove the warning or do I leave the code as is and accept that as it's a legacy system the code must work fine. Safety (and sadly professionalism) must win and in these situations I must leave the code as is until I get round to covering the relevant code with tests. However it's like a real stone in my shoe and I've got to ask: wouldn't it have been so much nicer if the original programmer simply looked at the warning and thought, "I don't think that's quite right...."?

So the moral of the story is please pay attention to your compiler warnings. They'll help you identify potential bugs in your code and they might even help write code that's easier to understand and consequently more maintainable.

Submit this story to DotNetKicks Shout it

In my professional life I'm finding it's more and more common for me to be working with a legacy code base. Whether that's because a system we've written is simply long-standing and has become legacy or whether it's because we've inherited a system to support and maintain, the problems you encounter are pretty similar. However I think it's worth pointing out one of the major differences, and why it's sometimes better to be involved in the latter situation rather than the former.

I'm a big fan of egoless programming but trying to separate people from their code can be hard to achieve. Indeed, if you were responsible for creating the system that has become legacy it can be extremely difficult to be entirely honest with yourself about what state the system is in. Now that's not to say people cannot admit their own mistakes, just that sometimes because your head is filled with all the history of why certain decisions where taken it can be difficult to look at things entirely objectively. Ultimately, the system is where it is and how it got there is relatively unimportant.

As it transpires though, I'm currently involved in working with a legacy system where I've inherited the code. The original developers are gone and whilst their experience and skills are most definitely missed, their absence allows a certain amount of freedom in moving things forward.

I think being able to look at a system without the baggage that often builds up helps in two ways:

  1. You don't have any pre-existing priorities about the things you want to fix - I know that whenever I deliver any system there are always things I'm not happy with and this can sometimes cloud my judgement about what is really important.
  2. You don't worry about upsetting anyone - I know from painful experience that developers can be sensitive souls and sometimes they simply cannot be separated from their code.

So from my experience I'm convinced that it's always nicer not to have had a hand in the original system development if you're going to moving it forward...

Submit this story to DotNetKicks Shout it

I've just recently download Windows Live Writer to help me out with my blog posting.... maybe it'll make me write more stuff! 

This is just a wee test post to make it works like I want.

Submit this story to DotNetKicks Shout it

NAnt vs MSBuild

I like to use NAnt for building my .Net projects.

I am not a huge fan of the IDE (Visual Studio), but I am well aware that I am in the minority here.

So if I plan to use NAnt on a project, I've got to do it in such a way that the other developers can still use the features of the IDE, mainly intelli-sense and the Error List window.

I've been using a very small .csproj file (described below) which calls out to NAnt, and a small custom MSBuild task to log errors so that the IDE will display them correctly.

Adding the source-code

Starting with a blank project file, you can add all the C# files with a single MSBuild entry like this:

<Compile Include="**\*.cs" />
        

And that's it! Now you can browse all your C# code using the solution explorer. The intelli-sense will recognise your classes and start auto-expanding as you type.

I typically add nodes for each file-type I want to be able to edit: .cs, .build, .xml, .config, etc.

Intelli-Sense

The IDE will use intelli-sense for the classes defined in your C# code files. You also want it to pick up external references. Add a line for each reference you need intelli-sense for in another ItemGroup:

<Reference Include="NHibernate" />
        

The IDE will look for the assemblies under the path defined in the <OutputPath> node near the top of your project file. If you want, you can add a <HintPath> sub-element:

<Reference Include="NHibernate">
    <HintPath>SDKs\NHibernate\NHibernate.dll</HintPath>
</Reference>
        

Now you'll have intelli-sense for your referenced assemblies too.

Running NAnt

When Visual Studio builds it runs the 'DefaultTargets' for the project (usually an MS-defined target named 'Build'). When Visual Studio cleans it runs a target named 'Clean', and when it re-builds it runs the target named 'Rebuild'.

You can change the 'DefaultTargets' for the project at the top of the project file:

<Project DefaultTargets="NAntBuild" ...
        

Then add a target that uses the Exec task to call NAnt:

<Target Name="NAntBuild">
  <Exec Command="SDKs\nant-0.85\bin\NAnt.exe" />
</Target>
        

Now the solution will build using NAnt. Add targets for 'Clean' and 'Rebuild' too if you want to do these via NAnt.

Note, when you customise your targets the IDE will give you a security warning. Since you customised it yourself, you can safely ignore this warning.

Error List

So far this works, but build errors are only found by examining the output window. It would be nice to have errors reported in the 'Error List' window so that you can just double-click them.

We wrote a custom MS-Build task called ExecParse, which inherits from 'Exec', to allow parsing of of the output and logging errors to the MS-Build engine.

The configuration for ExecParse takes a regular expression to search the output for, and allows logging of errors using text from captures in the regular expression.

A typical compiler error looks like:

[csc] c:\...\MyFile.cs(24,60): error CS0246: ...

So we can create a regular expression that captures this, and log an error using the MSBuild engine:

<UsingTask AssemblyFile="...\ExecParse.dll"
           TaskName="ExecParse.ExecParse" />
...
<PropertyGroup>
  <ExecParseConfiguration>
    <Error>
      <Search>\[csc\] (.*?)\((\d+),(\d+)\): error …
        ([^:]*): (.*?)[\n\r]</Search>
      <File>$1</File>
      <LineNumber>$2</LineNumber>
      <ColumnNumber>$3</ColumnNumber>
      <Subcategory>$4</Subcategory>
      <Message>$5</Message>
    </Error>
  </ExecParseConfiguration>
</PropertyGroup>
...
<Target Name="NAntBuild">
  <ExecParse Command="SDKs\nant-0.85\bin\NAnt.exe"
           Configuration="$(ExecParseConfiguration)" />
</Target>
        

Now you have a NAnt build, building within the IDE, using intelli-sense, and logging errors to the 'Error List' window.


Some links:

Submit this story to DotNetKicks Shout it

Introduction

NHibernate uses an Identity Map to maintain a single instance of each persistent entity in memory. This allows you to run round a domain model without having to worry about which instance of an entity you are modifying.

NHibernate also uses Lazy Load. The default implementation for this uses Dynamic Proxy (DP) to generate a class at runtime that inherits from your real class. This generated class intercepts all of the overridable methods and properties in your class to load the instance from the database if required.

What Identity Map Allows

Identity Map allows you to load up the same object twice but to have your references point to the same underlying instance.

The following example NUnit test demonstrates loading 3 objects, two of them with an ID of 1, and the other with an ID of 2:


[Test]
public void TestIdentityMap()
{
    MyClass myInstance1 = Session.Load<MyClass>(1);
    MyClass myInstance2 = Session.Load<MyClass>(1);
    MyClass myInstance3 = Session.Load<MyClass>(2);

    Assert.AreSame(myInstance1, myInstance2);
    Assert.AreNotSame(myInstance1, myInstance3);
}
        

The following diagram shows how the references relate to the objects created on the heap.

The Identity Map ensures that there is only one instance of the object with ID equal to 1 in memory.

Under the hood of NHibernate

The basic principle of Identity Map is easy to pick up. However, there is a further complication because of the implementation of the proxies used for Lazy Load.

When you ask NHibernate for an object, it returns you a instance of a class created at runtime by DP. This class intercepts any overridable methods/properties and redirects them to a second object instance that has the 'real' class type.

So in the above example, the following is actually created on the heap:

Limitations of the current implementation

Since there are really two instances created on the heap when NHibernate gives you a proxy, there is an identity problem that you could run into.

Problem 1, the 'this' pointer:

Consider the following class:


public MyClass
{
    public virtual bool IsMe(MyClass anotherMyClass)
    {
        return anotherMyClass == this;
    }
}
        

The following test will fail because the reference myInstance points to the proxy, while the 'this' pointer is the real class:


[Test]
public void TestIsMe()
{
    MyClass myInstance = Session.Load<MyClass>(1);

    Assert.IsTrue(myInstance.IsMe(myInstance));
    // The above line will fail
}
        

It is easy to imagine an example where a child entity notifies its parent of an event, and the parent then updates all its children except the child that initiated the event. The parent or child might have to be careful about reference comparison in such an example.

Problem 2, private methods/fields:

NHibernate relies on DP intercepting access to an instance of a class, and redirecting these calls to the 'real' instance. However DP is powerless to intercept private field and method access.

Situations where this could occur might be:

  1. sibling objects of a common parent object that communicate with each other;
  2. parent/child objects of the same type that communicate with each other (e.g., a tree);
  3. static methods that reference an instance of the same type.

Consider the following class:


public MyClass
{
    private int _calculation = -1;

    public virtual int Calculation
    {
        get { return _calculation; }
    }

    public static void SetCalculation(
                                MyClass instance,
                                int calculation)
    {
        instance._calculation = calculation;
    }
}
        

The following test will fail because the static method sets the field not on the 'real' class but the proxy object, while the property accessor is intercepted and redirected to the 'real' instance.


[Test]
public void TestCalculation()
{
    MyClass myInstance = _session.Load<MyClass>(1);
    MyClass.SetCalculation(myInstance, 7);

    Assert.AreEqual(7, myInstance.Calculation);
    // the above line will fail
}
        

It is not uncommon for a class to have an interface for talking to other instances of itself while keeping that interface private from outsiders.

Conclusion

I suspect NHibernate could be improved to make the proxies self-proxy. If this could be done then there would be no second instance created, and no potential identity problem.

To avoid problems with the 'this' pointer, simply try to avoid using it for comparison with another object.

To avoid problems with one instance of a class talking to another instance of the same class, prefer virtual methods over non-virtual ones to allow Dynamic Proxy to intercept any calls made on the instance. (i.e., avoid using private fields or methods from one instance to another instance of the same class.)

A complete example of the above, with failing NUnit tests, can be downloaded here: NHibernateProxyDemo.zip

Submit this story to DotNetKicks Shout it