Friday, September 20, 2013

Should I comment my code

Yes.... but.

I've been doing some reading regarding the pro's en con's (actually it is more like the for and against commenting) and I figured I'd make a quick summary of what I've come across.

Firstly I think that both camps (for and against comments) have the same goal in mind; readable code. The against camp's main concern seems to be:

  1. You don't need comments because your code should be structured in such a way to make it clear what it is doing.
  2. Lack of discipline causes redundant comments when code is updated
The "for people" says:
  1. Not commenting code is arrogant since you believe that what is apparent to you is apparent to everyone, or that the way you implemented the code is makes it obvious and readable.
  2. It is hard to know the intent of the code, especially when business logic comes into play
I actually think both camps have valid points, I had a bit of a thought experiment about it which I will show my thinking.

Enter code example 1 (A basic event scenario):

 public void OnStudentCreated(Student student) {  
   CheckSchoolCapacity();  
   if (student.HasSpecialNeeds) {  
       ConfirmSchoolHasSpecialNeedFacilities(student);  
   }  
   if (!AllocateStudentToClass(c)) {  
     NotifySchoolBody();  
   }  
   else {  
     SendEnrolledEmailToParent(c);  
     SendEmailToTeacher(c);  
   }  
 }  

Now as you can see, this is an event that fires when a student is created for a school application. There are various things to check and people to notify when this happens.

Also note I have coded it without any comments. So I then tried to think about what part of this could I not sufficiently make clear in the source code.

  • CheckSchoolCapacity() method, I can't say anything more about. I could possibly ask capacity of what? But unless you don't know what a Student and a school is, it should be pretty apparent.
  • Next is the SpecialNeeds section. Here I was of 2 minds about a comment. It is easy enough to go look at the implementation of the ConfirmSchoolHasSpecialNeedFacilities method but I currently don't know what will happen if the school doesn't have facilities. Does it throw an exception if the school doesn't, does it simply notify someone. You can either change the method name to something more specific, like EmailSpecialNeedRequirementsToStudentBody(student) or WarnIfSpecialNeedFacilitiesAreNotMet, or there may be some legal requirements that it will be looking at, OR it could be all of the above.
  • Similiar to the above point I didn't know what NotifySchoolBody meant, is it an email, a update to an internal system, both, etc. Again you can go look at the implementation but you can't see it at a glance.
  • I think the last to method calls are pretty self explanatory.
So now I had introduced another question, does a every method have to fully document its intent. In certain OO instances where a method may be an abstraction or partial, I will have no choice but to go look at the code inside these methods, and high level comments will give me little more clue to the method's function than the name already does. So, the answer to my question is probably no.

At this point I am starting to lean towards no comments, because I've not been able to find a situation where it would be useful. Lets face it, even if you use the method summaries for documentation, will anybody ever really look at it... I actually think they will, I mean I do all the time of MSDN. I want to know what a class does or a method in that class does, more to the point I would like to know how it does it. 

Enter code example 2 (The API method):


 public void AssignStudentToSchool(Student student, School school) {  
    // I don't know what this code is because it is in a black box dll  
 }  

I am using the StudentAPI.dll which gives me some basic functionality to writing a school system. Now, I know this example is slightly moot in open source software APIs but even if I can see the code, maybe I'm not a super senior developer and I don't understand what it does, even if it only adds a record in the database, looking at the code I may see some statements doing something I have no idea what their function is... ok so I need a cup of coffee and about 2 hours to figure it out. So even-though Bjarne Stroustrup probably don't need comments here, some of us mere mortals might.

BUT WHAT ABOUT TESTING, SURELY THE TEST WOULD GIVE YOU A CLUE!!!

This question is valid for an open source API but a black box API would still require at least a method signature comment section. Also it may be difficult to determine which test relate to a piece of code, unless it is really well structured and loads of discipline practiced to make sure all the code is covered.

My humble opinion in conclusion

  • Add comments when it is important for a method's meaning and function needs to be clear at a glance. This may be internal policy or for important method that do something high risk and important.
  • If you have a black box or complex method that you intend to expose via an API, though you can say that giving comments to any complex method signature is important.
  • I will add another one that wasn't apparent from this example, but put a comment if you do something weird so the next poor developer will know what you were thinking!
  • Obviously also use comments for TODO comments in your code.
  • If your class or method name uses a term that is very domain specific, for example a plane part that no-one else even knows exists. Please tell the poor next guy coming from the banking sector what it is.