Sign in to follow this  
Lain Rivers

How would a professional comment this code?

Recommended Posts

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"][size="2"]
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] [/size][/font]

Share this post


Link to post
Share on other sites
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:

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

Or..

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

Ie, not useful at all.

Share this post


Link to post
Share on other sites
I used to comment like that, except worse. I'd comment things like:

[code]
// Add a and b
c = a + b;
[/code]


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:

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

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


I do this now:


[code]
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;
}
[/code]

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.

Share this post


Link to post
Share on other sites
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 [i]why[/i] and [i]how[/i], not what.
For instance, this is a useless comment to anyone who knows the first thing about programming:
[code]
if number % i == 0: #if number is divisible by i, increment count.
count = count + 1
[/code]
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:
[code]
# 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.
[/code]
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.

Share this post


Link to post
Share on other sites
[list][*]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 ?).[/list][list][*]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).[/list]
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 ;)

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
[source lang="python"] [font="arial, sans-serif"][size="2"]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] [/size][/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

Share this post


Link to post
Share on other sites
The most important part is giving things good names. As someone pointed out, `print_nth_prime' would have been a better name for your function. Even better, make the function return the n-th prime instead of printing it, and then call it `compute_nth_prime'. The code that calls it can decide whether to print the result or do something else with it. Code that computes things should not do I/O and vice versa.

The example of finding the n-th prime is too much of a toy to show you an effective use of comments. I'll just tell you what I normally do. There are basically three types of comments I write:
* A general comment describing a file, a class or a function. I only do this when the name of the class or function doesn't tell you everything you need to know about it. This type of comment is particularly important if there are non-trivial design decisions that the reader of the code might need to be aware of.
* A very brief comment describing what the next few lines of code do. A person that is looking for a particular part of the code should be able to quickly skip through the sections by just reading these comments.
* An explanation of a particularly obscure line of code (anything with a magic number in it, for instance).

Share this post


Link to post
Share on other sites
Frankly, I think that putting comments after lines like that is horribly ugly. If you're going to do that, at the very least make the comments line up to the same tab.


This is better.

[code]def nth_prime(n):

# 2 is the first prime, start searching at 3
prime_list = [2]
number = 3

# search until we have n primes
while len(prime_list) < (n):
count = 0

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

#if there were no divisors, this is prime
if count == 0:
prime_list.append(number)

number = number + 1

print prime_list[-1] [/code]

Share this post


Link to post
Share on other sites
Looking at this my new rule of thumb might be "commenting a single line of code is pointless and if you find it necessary to understand the line, you stuffed too much into that line".

I like comments above _blocks_ of code that describe what it does. Then I'd prefer if people just delete that comment and put the block into an appropriately named function. Or in short: I'm a fan of self documenting code.

Why? Because everyday I look at commented code where the comments and code don't match at all. Either the code was changed a dozen times without updating the comments or (even more comment) code was copy/pasted with all comments and only the code was adjusted. Also this:

[code]
//Function A
//long describtion of A and it's parameters and return codes
int FunctionA(...)

//Function A
//long describtion of A and it's parameters and return codes
int FunctionB(...)

//Function A
//long describtion of A and it's parameters and return codes
int FunctionC(...)
[/code]

Apparently programmers are just notoriously bad at keeping comments consistent and the older the code, the more confusing and misleading the comments will be. Solution: comment as little as possible and only where it is REALLY necessary (ie. the whole thing the code does really IS that complicated and can't be broken down).

Share this post


Link to post
Share on other sites
The vast majority of my comments are for doxygen, apart from those the only comments i use are for odd performance tweaks or "temporary" solutions. (of those the "temporary" solutions are the most common one)

Share this post


Link to post
Share on other sites
[quote name='A Brain in a Vat' timestamp='1313207835' post='4848504']
Frankly, I think that putting comments after lines like that is horribly ugly. If you're going to do that, at the very least make the comments line up to the same tab.


This is better.

[code]def nth_prime(n):

# 2 is the first prime, start searching at 3
prime_list = [2]
number = 3

# search until we have n primes
while len(prime_list) < (n):
count = 0

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

#if there were no divisors, this is prime
if count == 0:
prime_list.append(number)

number = number + 1

print prime_list[-1] [/code]
[/quote]

This.

Share this post


Link to post
Share on other sites
The purpose of comments is to convey information that isn't obvious from, or contained in, the code. Some might say that might indicate a problem with the code, but that's not always the case. The code contains what you're doing, but it doesn't contain the why. For example, if a line of code is part of a bugfix, it bloody better have a comment on it indicating this (and the bug reference number). Handling WM_ERASEBKGND by returning non-zero deserves a comment (and possibly even a link to the relevant MSDN article). If you do something in a way that looks stupid but you need to do it that way for compatibility with another program or a file format; that's a comment. If you're working around a known driver or OS problem - comment! If you're putting in a nasty ugly piece of code to resolve a confirmed performance bottleneck - you better have a comment explaining that.

In other words their target audience is yourself in 6 months time, or some future maintainer of your code. Because if you have anything in there that you or this person might potentially get a bad smell from in the future, and if you haven't explained why it's needed, then you're asking for trouble.

Share this post


Link to post
Share on other sites
[quote name='mhagain' timestamp='1313370398' post='4849178']
The purpose of comments is to convey information that isn't obvious from, or contained in, the code. Some might say that might indicate a problem with the code, but that's not always the case. The code contains what you're doing, but it doesn't contain the why. For example, if a line of code is part of a bugfix, it bloody better have a comment on it indicating this (and the bug reference number). Handling WM_ERASEBKGND by returning non-zero deserves a comment (and possibly even a link to the relevant MSDN article). If you do something in a way that looks stupid but you need to do it that way for compatibility with another program or a file format; that's a comment. If you're working around a known driver or OS problem - comment! If you're putting in a nasty ugly piece of code to resolve a confirmed performance bottleneck - you better have a comment explaining that.

In other words their target audience is yourself in 6 months time, or some future maintainer of your code. Because if you have anything in there that you or this person might potentially get a bad smell from in the future, and if you haven't explained why it's needed, then you're asking for trouble.
[/quote]

I wish I could +2 this. Comments on the first iteration of a function are usually just needed for general code documentation like Javadocs or Doxygen; you don't need to get away with commenting why code looks stupid because it should generally look like exactly what it does on it's first iteration. Generally these comments' styles will be outlined in coding guidelines that are project specific. Once you get a code base that's on it's 5th product iteration there will have been things that were done wrong previously that you don't have the time to fix. It is better to say, "If the save system worked in a sane fashion we could call that here, but it doesn't because of Z and we don't have access to that piece of the source code, so we have to call it after function X or something will go awry and corrupt the save data."

I've also noticed that sometimes more obscure conditionals that might not be obvious just by what they show are good places for comments. Conditionals that might be able to ignore extra checks by being in a specific game state that indicates the condition you really care about has been excluded/dealt with already. Something like, "If the team was not full before adding a new player we should the player's won't be in their optimal positions, so resolve their positions.". That's probably not a great example, but I couldn't think of a better one off the top of my head.

Share this post


Link to post
Share on other sites
[quote name='mhagain' timestamp='1313370398' post='4849178']
In other words their target audience is yourself in 6 months time[/quote]



Wrong!


The target audience is yourself.... on Monday.


srsly wtf was I thinking on Friday???!?!?!



I had students who's response to 3/6 months was "I'll be finished by then and I wont ever be looking at it again". We all know how that pans out....


Myself, I tend to forget what I did yesterday.

Share this post


Link to post
Share on other sites
[quote name='BLiTZWiNG' timestamp='1313385178' post='4849261']Wrong!


The target audience is yourself.... on Monday.[/quote]

Too true.

Back on topic: the only comment this code needs is something like "I assume that you know what a prime number is" ;)

Share this post


Link to post
Share on other sites
I've come to use two styles of commenting... in my headers if a function name or it's parameters aren't blatantly obvious. Sometimes it's just not worth it to have a function name that 40 characters long, so basically if I have to abbreviate a name I'll comment on what it is.

Then in my source, I comment chunks when they tend to span more than 6-10 lines of code. or use alot of place holders or once again abbreviated names.

[source]

// x and y are screen coordinates
float x, y;

// screenx and screeny are screen coordinates
float screenX, screenY;

[/source]

I would comment the first set of coordinates because it's not obvious what space the coordinates ly in, where as the second set is obvious that they refer to screen space so I would leave out that comment.

Of course a lot of commenting is situational for me... I've got slight OCD so I always stick to a format style of commenting, but I'm selective of when commenting is needed. I really don't like to bloat my code, I hate scrolling around a header file that got 3-5 lines of comments for every function. I don't want to have to buy a 40 inch screen just to get an overview of the file.

Share this post


Link to post
Share on other sites
Definitely comment your thoughts on how something should work rather than provide a line by line play of what's going on. Any programmer who has half a brain should be able to see roughly what the code is doing. The trick is not knowing [i]what[/i] the code is doing so much as [i]why[/i] it's doing it maybe with some explanation of the background requirement of the code to exist at all. Down the road business rules change, what was once considered immutable now has 15 variations etc and it's knowing [i]how[/i] to interact with those requirements is how a programmer structures their work. Knowing that management considers a certain rule sacred could potentially save someone's head from the chopping block rather than writing out the fact that you're doing either concatenation or addition because you used a plus sign, which is irrelevant. Also, comment magic numbers or assign them to semi-descriptive constant variables.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this