Recently Jeff Atwood posted about Coding without Comments. Not only do I agree with Jeff’s post, I would actually like to see comments removed from code altogether. The other day I was working with the amazing FileHelpers library from Marcos Meli and needed to dig into the code to add some extra functionality. Now I’m not having a go at Marcos, or the library itself, but I’m just using this as an example because it highlights exactly the problem I have with comments in code:
#region " © Copyright 2005-07 to Marcos Meli - http://www.devoo.net"
// Errors, suggestions, contributions, send a mail to: marcos@filehelpers.com.
#endregion
#if ! MINI
using System;
using System.Data;
using System.Data.SqlClient;
namespace FileHelpers.DataLink
{
/// <summary>This is a base class that implements the <see cref="DataStorage"/> for Microsoft SqlServer.</summary>
public sealed class SqlServerStorage : DatabaseStorage
{
#region " Constructors "
/// <summary>Create a new instance of the SqlServerStorage based on the record type provided.</summary>
/// <param name="recordType">The type of the record class.</param>
public SqlServerStorage(Type recordType)
: this(recordType, string.Empty)
{}
/// <summary>Create a new instance of the SqlServerStorage based on the record type provided.</summary>
/// <param name="recordType">The type of the record class.</param>
/// <param name="connectionStr">The full conection string used to connect to the sql server.</param>
public SqlServerStorage(Type recordType, string connectionStr)
: base(recordType)
{
ConnectionString = connectionStr;
}
/// <summary>Create a new instance of the SqlServerStorage based on the record type provided (uses windows auth)</summary>
/// <param name="recordType">The type of the record class.</param>
/// <param name="server">The server name or IP of the sqlserver</param>
/// <param name="database">The database name into the server.</param>
public SqlServerStorage(Type recordType, string server, string database)
: this(recordType, server, database, string.Empty,string.Empty)
{
}
/// <summary>Create a new instance of the SqlServerStorage based on the record type provided (uses SqlServer auth)</summary>
/// <param name="recordType">The type of the record class.</param>
/// <param name="server">The server name or IP of the sqlserver</param>
/// <param name="database">The database name into the server.</param>
/// <param name="user">The sql username to login into the server.</param>
/// <param name="pass">The pass of the sql username to login into the server.</param>
public SqlServerStorage(Type recordType, string server, string database, string user, string pass)
: this(recordType, DataBaseHelper.SqlConnectionString(server, database, user, pass))
{
mServerName = server;
mDatabaseName = database;
mUserName = user;
mUserPass = pass;
}
#endregion
#region " Create Connection and Command "
/// <summary>Must create an abstract connection object.</summary>
/// <returns>An Abstract Connection Object.</returns>
protected sealed override IDbConnection CreateConnection()
{
string conString;
if (ConnectionString == string.Empty)
{
if (mServerName == null || mServerName == string.Empty)
throw new BadUsageException("The ServerName can´t be null or empty.");
if (mDatabaseName == null || mDatabaseName == string.Empty)
throw new BadUsageException("The DatabaseName can´t be null or empty.");
conString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
else
{
conString = ConnectionString;
}
return new SqlConnection(conString);
}
#endregion
#region " ServerName "
private string mServerName = string.Empty;
/// <summary> The server name or IP of the SqlServer </summary>
public string ServerName
{
get { return mServerName; }
set
{
mServerName = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
#endregion
#region " DatabaseName "
private string mDatabaseName = string.Empty;
/// <summary> The name of the database. </summary>
public string DatabaseName
{
get { return mDatabaseName; }
set
{
mDatabaseName = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
#endregion
#region " UserName "
private string mUserName = string.Empty;
/// <summary> The user name used to logon into the SqlServer. (leave empty for WindowsAuth)</summary>
public string UserName
{
get { return mUserName; }
set
{
mUserName = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
#endregion
#region " UserPass "
private string mUserPass = string.Empty;
/// <summary> The user pass used to logon into the SqlServer. (leave empty for WindowsAuth)</summary>
public string UserPass
{
get { return mUserPass; }
set
{
mUserPass = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
#endregion
#region " ExecuteInBatch "
/// <summary></summary>
protected override bool ExecuteInBatch
{
get { return true; }
}
#endregion
}
}
#endif
There are 167 lines of "code" in this example
However, if you remove the comments and regions and have a quick re-organisation of the code you can reduce this file down to 110 lines of just raw code:
#region " © Copyright 2005-07 to Marcos Meli - http://www.devoo.net"
// Errors, suggestions, contributions, send a mail to: marcos@filehelpers.com.
#endregion
#if ! MINI
using System;
using System.Data;
using System.Data.SqlClient;
namespace FileHelpers.DataLink
{
public sealed class SqlServerStorage : DatabaseStorage
{
private string mServerName = string.Empty;
private string mDatabaseName = string.Empty;
private string mUserName = string.Empty;
private string mUserPass = string.Empty;
public SqlServerStorage(Type recordType)
: this(recordType, string.Empty)
{}
public SqlServerStorage(Type recordType, string connectionStr)
: base(recordType)
{
ConnectionString = connectionStr;
}
public SqlServerStorage(Type recordType, string server, string database)
: this(recordType, server, database, string.Empty,string.Empty)
{
}
public SqlServerStorage(Type recordType, string server, string database, string user, string pass)
: this(recordType, DataBaseHelper.SqlConnectionString(server, database, user, pass))
{
mServerName = server;
mDatabaseName = database;
mUserName = user;
mUserPass = pass;
}
protected sealed override IDbConnection CreateConnection()
{
string conString;
if (ConnectionString == string.Empty)
{
if (mServerName == null || mServerName == string.Empty)
throw new BadUsageException("The ServerName can´t be null or empty.");
if (mDatabaseName == null || mDatabaseName == string.Empty)
throw new BadUsageException("The DatabaseName can´t be null or empty.");
conString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
else
conString = ConnectionString;
return new SqlConnection(conString);
}
public string ServerName
{
get { return mServerName; }
set
{
mServerName = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
public string DatabaseName
{
get { return mDatabaseName; }
set
{
mDatabaseName = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
public string UserName
{
get { return mUserName; }
set
{
mUserName = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
public string UserPass
{
get { return mUserPass; }
set
{
mUserPass = value;
ConnectionString = DataBaseHelper.SqlConnectionString(ServerName, DatabaseName, UserName, UserPass);
}
}
protected override bool ExecuteInBatch
{
get { return true; }
}
}
}
#endif
The thing that strikes me is that the code has been written so well that it’s perfectly clear what is happening in this class and you also have the added benefit of having a much more readable section of code.
Going back to the first sample of code, a lot of the original comments are what I would consider documentation. I agree that’s it’s important to document code, but is it really necessary to do it within the code itself?
I was discussing this with a co-worker the other day and we both agreed that a really cool feature in visual studio is to have some way of hiding comments within the IDE and placing a little token or tag in the margin of the page which would show the comments for the code when you hovered over them. Even better, would be to abstract comments out into another file, like code.comments.cs or code.comments.xml so that if people have a burning desire to comment on the code, they still can, but it just won’t be cluttering up the actual code files.
In reality, I’m still going to get beaten up in my day job for not having enough comments in my code, but I will still be pushing for less comments in code and better refactoring and documentation to support the code.
