On Code Comments
time to read 7 min | 1343 words

dJeff Atwood had a post about code comments, which I am completely agree with. One of the comments to the post caught my eye, talking about the assumption made when commenting:

It is not always feasible to have a programming guru on hand to fix every issue. Not everyone has the same skill set. Sometimes companies are stuck having to maintain code in languages their current staff aren't well-versed in. Gurus aren't available either at all in some areas, or for the money some companies have alotted for their IT staff.

I had posted before about this topic, but I think that this is an interesting take on the topic. When I comment, there are some assumptions that I make about the person reading the code:

  • That s/he knows the language, or is capable of learning it, for instnace, I tend to use this quite a bit:

    public string CacheId
    {
      get
      {
         return ( ViewState["CacheId"] ?? (ViewState["CacheId"] = Guid.NewGuid()) ).ToString();
      }
    }

    If the dev reading my code don't know what the null coalescing operator is for, or understand what expression chaining is doing here, then there is not chance they can follow the rest of the code. I don't write my code to be written by gurus, but I will not limit myself to using the basic features of a language just because a newbie will not understand them. Anonymous delegates is another thing that I like to use in many places With.Transaction(delegate), for instnace.
  • That s/he have at least a rudimetry understanding of the domain and the model. This is much harder than merely understanding the language/framework, by the way. If we will return to the HR model for a second, here is a piece of code that give the employee a 10% raise:

    SalarySnapShot raisedSalary = employee.At(startDate).Salary.Copy(startDate, endDate);
    raiseSalary.Amount *= 1.1m;
    emloyee.Salaries.AddOccurance(raiseSalary);

    There is a lot going on here. For instance, Copy will return a snapshot with a modified validity dates, and adding an occurance in the salaries will adjust the other salaries dates to fit the new salary (in itself a very complex problem). To me, the code is perfectly clear, and it is not hard to follow techincally, the problem arise when someone that doesn't understand the temporal model in use tries to follow it. I had a lot of problems coming to grips with it myself. I could put a comment there explaining why I need a temporal copy and what AddOccurance is doing, but this is a coment that would need to be repeated each and every time I touch a temporal object. I consider repeated comments a nasty code smell.
  • That they understand the technology:

    With.Transaction(delegate
    {
       Employee employee = Repository<Employee>.Find(empId);
       SalarySnapShot raisedSalary = employee.At(startDate).Salary.Copy(startDate, endDate);
       raiseSalary.Amount *= 1.1m;
       emloyee.Salaries.AddOccurance(raiseSalary);
    });

    This is a piece of code that will save the copy even though we never call Save() explicitly. I will sometimes throw a Save() anyway, just because it is clearer this way, but not always.

So, what do I comment?

I comment why I am not doing things when the situation is unique:

if(start > end )
  end = end.AddDays(1);// Need to make sure that the end date is after the start date, even though we are only using the time portion.

When there is a hidden catch:

group.AddUser(currentUser);//Will propogate the addition to all linked groups automatically.

Bug fixes:

employee.AddNote(newNote);
//We have leaking this pointer here, because the AddNote set newNote.Employee = this.
//this can cause problems when we group things by employee, so we do this explictly, to set to the proxy.
newNote.Employee = employee;

Tweet Share Share 0 comments
Tags: