Wednesday, January 26, 2011

Stop Being a Tourist!

About a month ago I saw a post on the Rails Best Practices site regarding comments and magic codes.  The point of the article is when you meta-program in Ruby you should write comments that reflect dynamically generated code.  If you don't want to read the full blog here is a simple example of what the author is suggesting you do:

I can remember having an argument with a friend and fellow engineer on commenting code.  I was trying to define the guidelines I use for writing comments when he blurted out one of the best remarks on commenting code of all time::
"If you need comments in order to able to read my code, then you're just a tourist" - Nick Marden
To me the example above illustrates the problem with many programmers and developers.  They are just tourists who are willing to visit someone else's code for example, but never really become a part of the code and learn how it really works.

So my mantra to other engineers is to stop being a tourist.  Get involved in learning how the code works instead of relying on complicated and immediately outdated comments.  Ruby has some fantastic tools that allow you to play with the code.  The first tool of course is irb which lest we forget its name stands for interactive ruby.  Let's take the above example and see how we can use irb to learn how the code works by putting the above class in a file and requiring it when we start the session:

The first thing we can do is verify that our object has been loaded and learn what else has been loaded into the object space.  Building a quick hash we find all of the classes that have been defined and of course our class is available.  I delineate this trick because it is useful when debugging hardcore problems but it is not the point of this blog.  Instead let's see what we can learn about the class without even looking at it's source.   Are there any class variables or methods defined?

From the above gist we can see the class methods that are defined that let us introspect on the variables that it has defined.  We see by called #class_variables we get an array of defined variables and this poor class has one defined.  From this we can also see what values it has defined for the lone class variable which is the last step in the gist.  Pretty amazing that without knowing anything about the source code to start off we can find out the author has the intention of keeping track of actions on a class basis.

Can we use this information to infer anything else?  Let's create the instance and see what we can learn from it:

The default printed version of the object as a string indicates we have instance variables which we can verify using the following:

We see we can learn what variables have been defined and then even better we can get the values of those variables.  When you combine this knowledge with the source code we now see the author wanted to create multiple variables to keep track of certain actions that can be part of an overall process.

This is a highly convoluted example but you can see the power of working with the code in irb is far more useful than trying to read comments that represent the interpolated code.  I think I would come up with the following guidelines for more effective comments in your code:
  • Business Rules - Define the business rules that are being implemented so the reader understands the implemented logic.  Far too often I see a lot of code with no business context explaining key rules that need to be followed and what are the consequences if they are not.
  • Algorithms - Explain to the reader why you chose a particular algorithm or implementation.  What I find is that I went through all the obvious choices but settled on the final one for a good reason so it is important to let everyone else know so they don't question the code.
  • Method/Script Usage - Always explain how to use the script and for methods give reasonable explanations of the arguments with allowed values and defaults.
  • Regular Expressions - Always explain the details of complex regular expressions.  I know I just made the argument for testing out the code and regular expressions are a great example of where irb is useful, but sometimes you just can't beat looking at written words that describe what a regex is doing.
The last rule I have is the most important rule:
  • Remind Yourself - Write comments with the idea of reminding yourself in the future why you did something.  How many times have you gone back and looked at your code from 6 months ago and questioned or rewritten it only to discover it was correct in the first place because you forgot all the valuable information used to make those decisions back then?  So avoid this mistake by writing comments with yourself as the intended audience.  Other members on your team will be thankful for it.
Got any more suggestions?  I would love to hear them but most importantly get in someone else's code and start learning!  In the next blog I will cover another way to learn how code works interactively that I use during development all the time so much so that my co-workers dubbed it DDD.   I'll cover it next.

1 comment:

  1. I was ready to write a long retort to your post until I got to the last section: your guidelines. I heartily agree with them.

    And I also would add to your guidelines something you may have intended but not specified: Comment immediately above the code on which you're commenting.

    However--and this might come as ironic from someone that grew up in Miami, FL--tourists aren't so bad!

    We're all tourists at times. Now matter how long you've been programming in Ruby (and Rails), you will always find yourself having to immerse yourself in some other group's code. Admit it, in an environment like RoR, there is so much that happens behind the scenes, it is often difficult to follow--even with an IDE and a "fast debugger" guiding your through every step.

    Case in point: I am writing an extensive extension to Spree right now and I often hear my college professors rolling over in their graves.

    Back then you never used a variable that wasn't defined within the same file, let alone a procedure that didn't even exist at load time! Can you spell resource_controller?? I think this is an example of code generation gone wild!

    My point is, that in this environment I would never criticize anyone for writing too many comments.

    but that's me... :)