Phil Bernie Consulting

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.

In so many aspects of our lives we have come to expect, and often demand, that not only are people qualified to participate in their industry, but they are required to gain approval from a central governing body before they can participate.

In Australia there are boards and registers of people that people need to be associated before they can participate in any of the following:

Medicine
Law
Accounting
Building
Architecture
Banking

And the list goes on.

Today I came across some code in a third party application that was truly horrible. It was littered with GOTO statements, was poorly structured and clearly demonstrated that the person that wrote it had a poor grasp on the basics of software engineering.

It’s not the first time I’ve come across this level of code and I’m not the only one. Sites like TheDailyWTF are built around exposing sub standard code for the enjoyment of others.

However it annoys me that by and large, within the software development community there is no real accountability. Sites like TheDailyWTF are amusing, but it also disturbs me how much WTF style code is in current production systems. More disturbing than the existance of the code, is that it seems to be accepted that the people that write this sort of code are able to rely on ignorance, poor interview techniques are the ability to straight out lie to move from position to position without too much trouble.

We’ve all heard of contracting horror stories where the candidate passes the interviews with flying colours, but when it comes time to fire up visual studio and debug some code, they’re completely clueless. By the time anyone realises the contractor is useless, the three months is up and it’s just easier to not renew the contract, cut your losses and move on than try and seek any sort of compensation.

But what happens to the contractor? They’re free to move on to another 3 or 6 month contract with their next unsuspecting client.

Not only do other industries not tollerate this sort of behaviour, in many cases it’s illegal, yet in software development, it’s more often than not accepted.

Even if companies wanted to do something after they are burned by these renegade contractors what can they do?

In law, students need to pass a "bar exam" before they are allowed to practice and even after they pass the exam complaints can also be made to the bar about unethical or unlawfully practicing lawers which can lead to the suspension and even the expulsion of lawers from practicing. In software, there is no such system which means that companies just have to suck it up when a hire goes bad.

I would love to see a "bar exam" system for software developers that provides instant transperancy to organisations looking to hire new developers, or in fact, any IT related discipline including database design and software architecture. The developer community is often the first to cry foul when a set standard isn’t met by a piece of software, yet when it comes to applying the same metrics to an individual or group of individuals claiming to be of a particular quality, there is no yardstick by which they can be measured.

One positive we can take out of no such system existing is that it ensures that there will be plenty of content available for TheDailyWTF for many years to come.

In any given apartment building there are two main groups that people will fit into. Owners and Renters. In general, it’s desirable to have a high number of people that own their apartments as opposed to a high number of renters as the owners as they’ve purchased their apartment so they’re committed for the long term. They take pride in their unit, as well as the general appearance of the building. They know that because they’re probably going to be in the building for a while they take care not to disturb the other tenants.

Renters on the other hand, are usually only temporary, they take less pride in their apartment and the building as they don’t feel any connection or affiliation to the building or the other tennants. If they annoy the other tennants, they don’t really care because it’s easier for them to just up and leave. They’re also not going to make any real investment into the building or their apartment because they get no benefit or value from that investment.

In software development you can apply some of these concepts to your codebase through code ownership.

What is code ownership?

Code ownership is a hard thing to define but I like to think of it like this. A project or solution is akin to an apartment building and within that project or solution, there will be manageable sections of code which will be akin to individual apartments. Allowing people to own those sections of code doesn’t mean that you write up a deed and hand it over to them, it basically just means that person is responsible for it. They primarily review the changes checked into that code, they are responsible for assessing the impact of any changes made to that code and they are generally in charge of overseeing that the code is well maintained.

What are the advantages of Code Ownership

There are many advantages to code ownership, but here are just a few:

1. Developers who own code take pride in the code. They are usually keen to ensure that the code they are responsible for reflects well on them so they aren’t as likely to allow just any old change to be checked into the code base
    
2. Developers who own code become intimate with the workings of that code so they are quickly able to identify any potential problems with the code as well as quickly debug and rectify existing problems with the code.
    
3. Code owners tend to fight for their code. They want to see their code enhanced, they want money spent on their code and resources dedicated  to their code so that they can continue to be proud of the code.
    
4. Code owners make it easier for others to learn about the code because they have already had a good amount of time invested in the code so it’s easier for new developers to learn from them than to go digging blindly through code.
    
5. By allowing people to own code, you give them a clear purpose within your team.

Code Ownership Isn’t for Everyone

Just like owning a house, owning code isn’t for everyone. It should be something that coders aspire to. You don’t necessarily need to have a formal process for allowing developers to become code owners, in fact, you’ll probably notice that the better developers in the team will often at times assume ownership of code – and that is the whole point. Within your team, you’re looking to keep the developers who are willing and able to take on code as their own. At the end of the day, you’re going to need some guys who will essentially be code renters. The more code owners you can get  means less code renters which means your team will be better off.

You should be wanting to own code

As a developer, you should be looking to own code within your code base. Even if your company doesn’t actively encourage code ownership, you should still look for areas where you can take ownership of code. By taking ownership of code, you’re also assuming responsibility and managers like that. Sure it can be a risk because if the code fails, it lands on your shoulders, but that isn’t always a bad thing either because you already have the domain knowledge which means you can probably fix it quickly. On the flipside if the code you own is maintained to a high standard, people will start to notice.

Owning code also increases your value to the team and the more pieces of code you own the more valuable you are.

At the end of the day, owning code isn’t for everyone however encouraging team members to own code is only going to benefit your projects in the long term by helping to reduce the number of code renters while at the same time it has the added benefit of giving team members something to aspire to.