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.
