Sunday, September 21, 2014

Constructor Parameters

It's funny, in all my years of writing software I have never given much thought to when I should use parameters in a constructor verses setter properties (or getXXX / setXXX methods for you Java people). I always just picked it based upon what I saw as convenience. Usually convenience to me meant less lines of code. For example:

MyClass myClass = new MyClass("hello world");

Is shorter than saying:

MyClass myClass = new MyClass();
myClass.Text = "hello world";

But the other day I was once again overriding the GetHashCode method for a .NET class because I had overridden the Equals method and you should override both at the same time.  Usually I just don't care much about this method and write some probably incredible poor hashing method (aka, GetHashCode of some properties)

Well, I decided to do a bit of a Google for a proper hash method. I actually found some very interesting proposals and looked to implement some of them.  All of the methods of overriding GetHashCode look at using the fields in the class to develop a unique hash.

Now, the important thing about the GetHashCode method is that the result from it must be immutable for the class.  Since we are basing our hash off of some class fields, those fields must now also be immutable.

Let's say you have a class called Person. It would seem reasonable that the hash code be based upon the person's name. Every class with a name of John Doe should generate the same hash code.

But here in comes the problem, what if the name changes? We want the GetHashCode to always return the same thing for the class and we need the name to be immutable now. This is especially important if that class is added to a list or becomes a key to a dictionary.

And so here I was lead to my "AH HA!" moment. Parameters passed in with the constructor should be considered immutable and define the class for its life time. Any constructor parameter should not be allowed to be changed after the instantiation of the class. This way the GetHashCode always returns the same value regardless. So, my person class gets a constructor of Person(string firstName, string lastName). Now forever going forward those two properties are fixed to the class and the GetHashCode always works.

Class fields that are accessed via set properties (or setXXX methods) are mutable and therefore cannot be used for hashing. So, for my Person I may have mutable properties, but the name is now considered fixed.

Person myPerson = new Person("Jane", "Doe");
myPerson.Slogan = "Hello World!!!";

In this case, the name is immutable, whereas the slogan is not.  The slogan cannot participate in hashing.

In Conclusion

I am proposing that class constructor parameters be immutable and used to define the class when implementing of the Equals and GetHashCode methods. Alternatively, getter/setter properties or methods for immutable properties should throw an exception if a calling method attempts to change the value.

A word of caution

All constructor parameters that participate in the hash code themselves must also be immutable.  Following this principle though out your classes will ensure that, but if parameter's object can change its hash code, then your classes hash code is no longer fixed.


Wednesday, May 14, 2014

Simplied reading of code

So I recently was working on refactoring a piece of code I found online. It was this:
if (this.itemUnderDragCursor == newItem)

// The first pass handles the previous item under the cursor.
// The second pass handles the new one.
for( int i = 0; i < 2; ++i )
    if( i == 1 )
        this.itemUnderDragCursor = newItem;

    if( this.itemUnderDragCursor != null )
        UIElement item = this.GetSelectorItem(this.itemUnderDragCursor);
        if( item != null )
            SelectorItemDragState.SetIsUnderDragCursor( item, i == 1 );
Now looking at this code, it's tough to tell what exactly it does. Refactoring it, same functionality, but much easier to read:
void UpdateItemUnderDragCursor(object newItem)
    if (this.itemUnderDragCursor != newItem)
        SetItemSelectedState(itemUnderDragCursor, false);
        itemUnderDragCursor = newItem;
        SetItemSelectedState(itemUnderDragCursor, true);

private void SetItemSelectedState(object itemToSetState, bool state)
    if (itemToSetState != null)
        UIElement item = this.GetSelectorItem(itemToSetState);
        if (item != null)
            SelectorItemDragState.SetIsUnderDragCursor(item, state);
Lesson here is that code can be made complex to read and maintain or can be made easy.