ailon's DevBlog: Development related stuff in my life

What's Wrong with this?

5/29/2008 9:55:30 AM

Every time I read a C# book or an article or just a code sample in MSDN I can't stop wondering why don't people (and by people I don't mean just average Joe's but respected authors, MSDN documenters, etc.) use this qualifier unless it's absolutely required to distinguish between identically named local variables and object fields?

I use this all the time and I think it's way easier to understand your own code. And don't get me started on someone else's code. Let me show you an example.

Yesterday I was implementing a SiteMapProvider for a project I'm working on. I looked  at the sample in MSDN. The main "meat" is in BuildSiteMap method. I'll paste it here in full. It's a little long but that's exactly the point.

// Build an in-memory representation from persistent // storage, and return the root node of the site map. public override SiteMapNode BuildSiteMap() { // Since the SiteMap class is static, make sure that it is // not modified while the site map is built. lock (this) { // If there is no initialization, this method is being // called out of order. if (!IsInitialized) { throw new Exception("BuildSiteMap called incorrectly."); } // If there is no root node, then there is no site map. if (null == rootNode) { // Start with a clean slate Clear(); // Select the root node of the site map from Microsoft Access. int rootNodeId = -1; if (accessConnection.State == ConnectionState.Closed) accessConnection.Open(); OleDbCommand rootNodeCommand = new OleDbCommand("SELECT nodeid, url, name FROM SiteMap WHERE parentnodeid IS NULL", accessConnection); OleDbDataReader rootNodeReader = rootNodeCommand.ExecuteReader(); if (rootNodeReader.HasRows) { rootNodeReader.Read(); rootNodeId = rootNodeReader.GetInt32(0); // Create a SiteMapNode that references the current StaticSiteMapProvider. rootNode = new SiteMapNode(this, rootNodeId.ToString(), rootNodeReader.GetString(1), rootNodeReader.GetString(2)); } else return null; rootNodeReader.Close(); // Select the child nodes of the root node. OleDbCommand childNodesCommand = new OleDbCommand("SELECT nodeid, url, name FROM SiteMap WHERE parentnodeid = ?", accessConnection); OleDbParameter rootParam = new OleDbParameter("parentid", OleDbType.Integer);
rootParam.Value = rootNodeId;
childNodesCommand.Parameters.Add(rootParam); OleDbDataReader childNodesReader = childNodesCommand.ExecuteReader(); if (childNodesReader.HasRows) { SiteMapNode childNode = null; while (childNodesReader.Read()) { childNode = new SiteMapNode(this, childNodesReader.GetInt32(0).ToString(), childNodesReader.GetString(1), childNodesReader.GetString(2)); // Use the SiteMapNode AddNode method to add // the SiteMapNode to the ChildNodes collection.

// LOOK HERE
AddNode(childNode, rootNode);
// LOOK HERE

} } childNodesReader.Close(); accessConnection.Close(); } return rootNode; } }

I've highlighted one line in the method:

AddNode(childNode, rootNode);

By glancing at that line could you tell me right away is rootNode a local variable or a field of the object? Wouldn't it be much more obvious if it was

AddNode(childNode, this.rootNode);

We loose readability by omitting this so we must gain something in return, right? What are we gaining? Do I miss something other than saving 5-10 bytes in source file and 5 extra keystrokes? And the keystroke argument isn't always true.

Suppose you have a class defined like this:

class test
{
    public XmlNode XmlNodeHolder;
    public test()
    {
        XmlNodeHolder = null;
    }
}

To type that XmlNodeHolder in the constructor with the help of IntelliSense I had to press 8 keys (x-m-l-n-o-d-e-h) before I got the right line in IntelliSense. Now I would use this

class test
{
    public XmlNode XmlNodeHolder;
    public test()
    {
        this.XmlNodeHolder = null;
    }
}

I could type this.XmlNodeHolder in 4 key presses (t-h-.-x). So the only argument that I see and can't argue with is saving bytes in the file. But who cares about 10 bytes in the source file these days? 100 bytes? 1kb anyone?

So either I miss something important in functionality, performance or something else or I just can't understand the trend.

kick it on DotNetKicks.com

Tags: ,

blog comments powered by Disqus
Copyright © 2003 - 2014 Alan Mendelevich
Powered by BlogEngine.NET 2.5.0.6