My Secret Life as a Spaghetti Coder
home | about | contact | privacy statement
Several days ago I wrote about common excuses people use for commenting code and things you can do to get rid of them. I took the stance that rarely is there something so complex that it really requires comments.

In other words, much of the time, comments are crutches for understanding bad code.

To show a counter-example where comments were needed, Peter Farrell posted a snippet of code that was hard to understand:

<cfloop condition="patArr[patIdxEnd] NEQ '*' AND strIdxStart LTE strIdxEnd">
  <cfset ch = patArr[patIdxEnd] />
  <cfif ch NEQ '?'>
    <cfif ch NEQ strArr[strIdxEnd]>
      <cfreturn false />
    </cfif>
  </cfif>
  <cfset patIdxEnd = patIdxEnd - 1 />
  <cfset strIdxEnd = strIdxEnd - 1 />
</cfloop>

Indeed, that code is hard to understand, and comments would clear it up. And I'm not trying to pick on Peter (the code is certainly not something I'd be unlikely to write), but there are other ways to clear up the intent, which the clues of str, pat, * and ? indicate may have something to do with regular expressions. (I'll ignore the question of re-implementing the wheel for now.)

For example, even though pat, str, idx, ch, and arr are often programmer shorthand for pattern, string, index, character, and array respectively, I'd probably spell them out. In particular, str and array are often used to indicate data types, and for this example, the data type is not of secondary importance. Instead, because of the primary importance of the data type, I'd opt to spell them out.

Another way to increase the clarity of the code is to wrap this code in an appropriately named function. It appears as if it was extracted from one as there is a return statement, so including a descriptive function name is not unreasonable, and would do wonders for understandability.

But the most important ways in which the code could be improved have to do with the magic strings and boolean expressions. We might ask several questions (and I did, in a follow-up comment to Peter's):
  1. Why are we stopping when patArr[patIdxEnd] EQ '*' OR strIdxStart GT strIdxEnd?
  2. Why are we returning false when ch=="?" and ch!=strArr[strIdxEnd]?
  3. What is the significance of * and ?
In regular expression syntax, a subexpression followed by * tells the engine to find zero or more occurrences of the subexpression. So, we might put it in a variable named zeroOrMore, and set currentPatternToken = patArr[patIdxEnd]. We might also set outOfBounds = strIdxStart GT strIdxEnd, which would mean we continue looping when currentPatternToken NEQ zeroOrMore AND NOT outOfBounds.

Similarly, you could name '?' by putting it in a variable that explains its significance.

And finally, it would be helpful to further condense the continue/stop conditions into variable names descriptive of their purpose.

In the end, regular expression engines may indeed be one of those few applications that are complex enough to warrant using comments to explain what's going on. But if I was already aware of what this piece of code's intent was, I could also have easily cleared it up using the code itself. Of course it is near impossible to do after the fact, but I think I've shown how it might be done if one had that knowledge before-hand.

What do you think?

Hey! Why don't you make your life easier and subscribe to the full post or short blurb RSS feed? I'm so confident you'll love my smelly pasta plate wisdom that I'm offering a no-strings-attached, lifetime money back guarantee!


Comments
Leave a comment

The idea of comments is to help other developers understand the section of code clearly, whether you like to admit to that or not, reading a comment is actually easier than reading code to understand what it does.

If you are in a position where more than one developer might touch the code, then comments help understand why and how the code does what it does, much quicker than reading lengthy snippets of code.

Posted by Andrew Scott on Jan 25, 2013 at 11:33 AM UTC - 5 hrs

This is a hard one. I think I don't comment as much as I should, but I typically try to make my code sufficiently clear that it does not need comments (at least on the mechanics of the code).

What I do try to comment, is the "why" of the code. On the other hand, I hope to have that mostly addressed in my test code.

That being said, perhaps I should address it in the code directly as well. I just worry that my comments will become stale if and when I fail to update them for changes.

Posted by Steve Bryant on Jan 26, 2013 at 10:53 PM UTC - 5 hrs

@Andrew Scott: I can't think of any well-written code I've seen where I felt like English would be any easier to understand than the code itself. This is particularly true when the comments become out of date and get left around because everyone fears losing whatever value they once had.

You mention reading lengthy snippets of code as a place where comments can be useful, and I agree. But I would rather have shorter snippets of code that do one thing well, with good names. If I have that, then comments get in my way of understanding.

@Steve Bryant: I agree. Most times for me the why is straightforward, but when (for example) you have to do something that doesn't seem like the most obvious way to do something, then a comment explaining why you made that decision is very useful.

As an example, consider some code where the straightforward way may be a simple nested loop, but it runs slow in production because you're dealing with a lot more records than you are in development. So you go around the problem with a different strategy to improve the performance. Leaving a comment there as to why you made that decision is important so someone (perhaps yourself!) doesn't come back later and think "this code is obtuse, let me make it easier to understand" and ends up back at the same slow implementation you had before.

Posted by Sammy Larbi on Jan 27, 2013 at 08:55 AM UTC - 5 hrs

Documentation-generation is another example of helpful comments, IMO. I should have mentioned it as well. =)

Posted by Sammy Larbi on Jan 27, 2013 at 08:56 AM UTC - 5 hrs

I can very easily come up with many.

One right of the top of my head is that there are so many problems with ColdFusion syntax, especially when dealing with running your code on Railo or openBD or just to provide a work around.

Now I do a lot of unit tests, but unit tests don't cover these type of problems, so if you are working with Open Source or even having other developers work on your code, then comments will tell the developer why a piece of code is written the way it is.

Now if you are not 110% familiar with those problems, a junior developer may never know, and just rewrite the code without the knowledge of why.

Comments would stop this!

Posted by Andrew Scott on Jan 27, 2013 at 09:18 AM UTC - 5 hrs

Andrew,

I try to code such that comments are not needed and I think good unit tests goes a long way in that effort. Even with that, there are still situations where comments are essential.

Working around oddities and errors in different systems definitely falls among them. In part, because if you note that you are working around an issue in IE5 or in ColdFusion 4 (by way of examples) then future coders could find that they *could* rewrite the code in a simpler format.

Posted by Steve Bryant on Jan 27, 2013 at 08:40 PM UTC - 5 hrs

That's the issue you try code.

Let's take an example without comments which one do you think is the correct code?

var test = {arg1 = "test", arg2 = "another one"};
myObejct.method(argumentCollection = test);

compared too

myObejct.method({arg1 = "test", arg2 = "another one"});

Now let's put comments into the correct code.

// Due to ColdFusion sometimes having issues with struct
// definitions defined in the method, we need to define it
// outside and pass it this way.
var test = {arg1 = "test", arg2 = "another one"};
myObejct.method(argumentCollection = test);

Without the comments, another developer will come along and possibly rewrite this and break so much without knowing that is what has happened.

There are so many differences like this between Railo and ColdFusion that unit tests should not cover, and have no need to cover.

At the end of the day as developers we do what we need to do, to make our lives easier, not just for us but anyone else who might touch the code.

The reason this should not be covered in a unit test, because a unit tests only job is to pass and make it fail or pass, a unit test doesn't and shouldn't care about what is in the body.

The method body is where we NEED to place comments for everyone to know why something like this is done.

But like I said if your way works, and means your productive with people refactoring this and breaking it without knowing then I am happy for you.

I just watched a video on Home Automation, where there is so many non standard systems in place to make this work, it is a requirement that it be totally documented so that anyone picking it up, will take 5-10 minutes to fix the problem. Without it you are more likely to have the contractor charge you the $100 for coming out and walking away, or it costs you another $500 to do a 5 min job because he spends more time trying to work out where the problem has arisen.

So again if you feel you don't need comments, then be prepared for the problems later.

Posted by Andrew Scott on Jan 28, 2013 at 12:40 AM UTC - 5 hrs

Andrew,

Not sure if you were responding to me, but I was agreeing that those are situations where you would need comments.

Posted by Steve Bryant on Jan 28, 2013 at 06:57 AM UTC - 5 hrs

Yeah I was, but it sounds like I misunderstood you then.

Posted by Andrew Scott on Jan 28, 2013 at 06:58 AM UTC - 5 hrs

Leave a comment

Leave this field empty
Your Name
Email (not displayed, more info?)
Website

Comment:

Subcribe to this comment thread
Remember my details
Google
Web CodeOdor.com

Me
Picture of me

Topics
.NET (19)
AI/Machine Learning (14)
Answers To 100 Interview Questions (10)
Bioinformatics (2)
Business (1)
C and C++ (6)
cfrails (22)
ColdFusion (78)
Customer Relations (15)
Databases (3)
DRY (18)
DSLs (11)
Future Tech (5)
Games (5)
Groovy/Grails (8)
Hardware (1)
IDEs (9)
Java (38)
JavaScript (4)
Linux (2)
Lisp (1)
Mac OS (4)
Management (15)
MediaServerX (1)
Miscellany (75)
OOAD (37)
Productivity (11)
Programming (168)
Programming Quotables (9)
Rails (31)
Ruby (67)
Save Your Job (58)
scriptaGulous (4)
Software Development Process (23)
TDD (41)
TDDing xorblog (6)
Tools (5)
Web Development (7)
Windows (1)
With (1)
YAGNI (10)

Resources
Agile Manifesto & Principles
Principles Of OOD
ColdFusion
CFUnit
Ruby
Ruby on Rails
JUnit



RSS 2.0: Full Post | Short Blurb
Subscribe by email:

Delivered by FeedBurner