How would a professional comment this code?

Started by
29 comments, last by Vectorian 12 years, 8 months ago
Hello, I'm just curious (as my commenting skills are horrid) how someone with some experience would comment some code.

For example code I'll just throw up my most recent text book assignment for computing the nth prime number. Any help would be appreciated for good commenting!

[source lang="python"][font="arial, sans-serif"]
def nth_prime(n):

prime_list = [2] #Skip 1 and 2 by entering what is already known; 1 is not prime, 2 is prime.
number = 3 #Starting point is 3

while len(prime_list) < (n):
count = 0

for i in range(2,number):
if number % i == 0: #if number is divisible by i, increment count.
count = count + 1

if count == 0: #if number was not divisible by any i, number is prime, add number to prime_list and increment number
prime_list.append(number)
number = number + 1
else: #if number was divisible by any i, number is not prime, increment number
number = number + 1

print prime_list[-1] #print out last prime of prime_list
[/source] [/font]
Advertisement
I've seen professional code commented like that. Why would anyone comment a print statement to tell the reader that it does printing? Isn't that what self-commenting code is? Bleh.It's about as useful as this:

for (int i = 0; i < 5; i++) // Will loop five times!

Or..

int number = 5 + 2; // This is addition!

Ie, not useful at all.
I used to comment like that, except worse. I'd comment things like:


// Add a and b
c = a + b;



That's where it's going too far.

For what it's worth, I think your code is commented fine. I strongly prefer to read comments to get an understanding of what a piece of code is doing instead of trying to decipher variable names and operators. My eyes naturally gravitate toward comments first, so if your code is well commented and tells the story of what a function does within reason, I'm a happy programmer. If your code is well formatted and variables are named appropriately, then you can get away with less comments, I think. That's what I've worked toward recently.

So, instead of:


char buff1[256];
char buff2[256];

// Copy buff2 into buff1 if foovar_1 is true
if (copy) copyBuffConts(buff1, buff2, 256);
else return;



I do this now:



char input_buffer[256];
char output_buffer[256];

if (need_to_copy) {
copy_buffer(input_buffer, output_buffer, 256);
} else {
// Explain why you'd want to return here instead of the end of the function
return;
}


To me at least, that reads easily enough to not need much in the way of comments.

I'll agree about the print line though. Commenting peculiarities (such as starting at an index other than 0) or comment control flow points (i.e. at if or switch blocks) is probably a reasonable compromise. Any out of place or offensive looking code (such as pointer arithmetic) should probably be justified with commenting.
Success requires no explanation. Failure allows none.
I imagine everyone has different ideas on what specifically should be done while commenting, but I think we can agree that in general you need to make comments to say things that the code doesn't.

You should describe why and how, not what.
For instance, this is a useless comment to anyone who knows the first thing about programming:

if number % i == 0: #if number is divisible by i, increment count.
count = count + 1

I know var = var + 1 means increment. What I didn't know until I read the whole example was that the code checks to see if n is prime by trying every number from 2 through (n-1), counting up divisors. A comment like the following would tell me immediately how this code behaves:

# This part here tries every number from 2 through (n-1), counting up divisors of n.
# If there aren't any, then the n is prime.

If I care about details, then I can read the code. Even without doing that, though, I have a good idea of what is in the function after reading only two lines. That's what you try to do with comments, reduce the amount of effort required to understand what code does.
  • The first comment is probably incorrect: it's not because you know that 1 and 2 are primes that you treat them specially (why stop at 2 in such case, we also know that 3 is prime). It's more because the general method wouldn't work for them (or would it ?).
  • You could dedicate a function to the for loop. By naming it correctly (something like is_divisible_by), you remove the need for a comment.
  • "count" is not a very good name, because we don't know what it counts. Actually, you even use it as a boolean.
  • As said by others, no need to comment the obvious print statement (people reading the code are supposed to know the language).

A few things could be said about efficiency too: you are searching for the Nth prime, so you don't need to store all the primes up to that Nth. Or you can store them, but then use a more efficient algorithm.
Your average "professional" doesn't comment hardly at all, unfortunately -- Now, your consummate professional, them's where its at.

Seriously though -- comments should never repeat code -- plain and simple. You're just begging for the comments to get out of sync with the code, and the only thing worse than no comment at all is a comment which *is lying to you.*

Function/class/module -level summary comments are popular and useful (though you still do have to be wary of these getting out of sync as well). A log of important changes or functionality decisions can be good (though perhaps are better off in the design doc or a wiki). Generally, you should be describing things at a very high level ("this is the algorithm this function implements, and here is the whitepaper") or telling the reader why the code is the way it is ("Normally I'd do X, but I can't here because Y" or). Particularly you should call out anything which you have found to be a false assumption, or which alternate paths you tried and why they failed.

Comments should tell you where the programmer has traveled before, why they left for greener grass, and where they're train of thought is heading.

throw table_exception("(? ???)? ? ???");

Commit log: #2412

Closed ticket #41223, fixed #41103, 40021, 39453
Generally, I find half my comments being in the train of: "//TEMP HACK!!!!!!! REPLACE!!!!" or somesuch. Funny, I'd say about 75% of those "temporary hacks" never get around to being replaced - they just linger on until the game sees the light of day; or they start causing trouble ;)
"I will personally burn everything I've made to the fucking ground if I think I can catch them in the flames."
~ Gabe
"I don't mean to rush you but you are keeping two civilizations waiting!"
~ Cavil, BSG.
"If it's really important to you that other people follow your True Brace Style, it just indicates you're inexperienced. Go find something productive to do."
[size=2]~ Bregma

"Well, you're not alone.


There's a club for people like that. It's called Everybody and we meet at the bar[size=2].

"

[size=2]~

[size=1]Antheus
The OP's example doesn't need any comments, but the function should be renamed to 'print_nth_prime'.

Perhaps the only comment I would add is a warning that it's a VERY slow prime-finding algorithm.
[source lang="python"] [font="arial, sans-serif"]def nth_prime(n):
first_prime = 2
prime_list = [first_prime]
possible_prime = first_prime + 1

while len(prime_list) < (n):
if is_number_divisible_by(possible_prime, prime_list) :
possible_prime = possible_prime + 1
else:
prime_list.append(possible_prime)

print prime_list[-1]


def is_number_divisible_by(number, divisor_list):
count = 0
for i in divisor_list:
if number % i == 0:
count = count + 1

if count == 0:
return False
else: return True[/source] [/font]

[s]I don't know python, so I apologise if it's nonsense.[/s]

Worked out the kinks on codepad.org


I'd actually make further changes too, but they'd have other effects besides replacing comments. Specifically, renaming the original function to better reflect what it does, which would break any code using the current one. Also, removing the count variable, which is confusing because of how pointless it is.

I think I accidentally changed it slightly, though. Not sure if it still works.


[quote=SriLumpa]You could dedicate a function to the for loop. By naming it correctly (something like is_divisible_by), you remove the need for a comment.[/quote]
Ha. It seems great minds think alike. :P

This topic is closed to new replies.

Advertisement