Which is better, one central detailed comment, or several smaller ones?

Question

To restate the question: Which is better, one central detailed comment, or several smaller "sprinkles"?

Bear with me while I set up the scene:

I've got this function for popping a value from a queue implementation:

int queue_pop(queue *list, void **pValue);

If you're familiar with C, you understand that calling this function like this:

struct foo *pFoo;
int rc;
rc = queue_pop(g_FooList, &pFoo);

you get this:

gcc: warning-incompatible pointer type passing 'foo **' to parameter of type 'void **'

Now, as it happens, calling this function like this is perfectly safe; the function never addresses the dereferenced pointer as anything other than a void*, and memory allocation and copying is done based strictly on the element size, which is defined on any given queue. But, to avoid the warnings, I call it like this:

rc = queue_pop(g_FooList, (void**)&pFoo);

which is also not strictly portable in C, but again, in this case, is perfectly safe. But, since casting function arguments is almost NEVER necessary in C, and is in fact almost ALWAYS covering up an error, I document the first call of this type in every function like this:

/* Explicit cast - see comments on queue_pop function in queue.c */
rc = queue_pop(g_FooList, (void**)&pFoo);

That way, anyone reading the code can open the queue.c file and read a detailed explanation of why this is OK, and not to worry about it. And, since they won't do that, at least they know that it was done on purpose, and there is a reason.

The question arises because somone suggested that I get rid of the cast -- since it is so unusual -- and just call the function with the concrete pointer type. Then suppress the warning with a #pragma push at the top of the source file, and put a detailed comment on the pragma explaining the suppression. Since the codebase is eventually going to be open-source, and the source code comments (versus header file comments) are therefore going to have a wider audience than normal, it's more organized that way.

Aside from the fact that I'm not a big fan of suppressing warnings in the first place (except for some of the sillier MSVC ones), it's been my exprerience that:

• A) people will not read
• B) if you want someone to read something, you better stick it right in front of their eyeballs
• C) see A

Now, the comments tag says it's for questions about "Best Practices", but I realize this one might tend a bit too far toward "opiniony". If that's the case, just hang a few downvotes on it, and I'll delete the question.

EDIT I saw this question, but it looks like a general mosh about whether or not to use comments at all. That's not the issue here: I'm definitely going to use some kind of comment.

Answer 1

As a side node, you can define a macro:

    /* pop_macro is used to avoid the incompatible type compiler warning */ 
    #define pop_macro(FooList,Foo) queue_pop(FooList, (void**)Foo)

As opposed to:

    queue_pop(g_FooList, (void**)&pFoo)

it would be like:

    pop_macro(g_FooList, &pFoo)

and the compiler warning will disappear and there is a single spot to make a comment that other people can find.

Answer 2

The answer is: whichever your fellow programmers will read.

In general:

Sprinkled comments will be read more because humans have short attention spans and a comment on a line we are already reading tends to get noticed. We will rarely (if ever) go searching for a comment, but we might just notice it when it's required. We'll only read it if we already know it exists and if it's not too difficult to find.

The main time a central comment will get read is during a code review. This can be very useful, as it can make the review for a particular edit / branch / feature simpler and more straight forward. You can then reference that central comment throughout the code, backing up your sprinkles.

Also any standards you have set - such as function explanations in header files - may be expected / desired by your fellow programmers, if they're accustomed to it.

In the end, most programmers will not read every comment, especially in the open-source world where they almost certainly have not taken the time to read through documentation or browse the entire code-base before diving in. It's better to have the information right in front of you, even if it is a short reference to a bigger explanation, at least then you know there is an explanation and this wasn't some mistake.

More specifically to your example:

If you can resolve a compiler warning/error with code, you should do that. Explain how you are doing it, but solve it in the code not with compiler settings. Someone may want to wire up a different compiler one day and having all of your #pragma could be a waste of time. In that way, a short comment is a wonderful thing.

Answer 3

A disadvantage of having a code comment every time you call it is there are so many comments the user will begin ignoring them. Consider creating a function which you call like my_queue_pop(g_FooList, &pFoo); and which itself then calls queue_pop(g_FooList, (void**)&pFoo);. Then put the comment inside the definition of my_queue_pop. That way the reader will understand from just looking at the name of the function.

Answer 4

The accepted answer pretty much dovetails with my initial approach. It occurs to me that since this is destined to be an open source codebase, which will presumably be built from source as needed, I could take @Robert Harvey's advice and just leave the warning, and let the end user worry about it if it bothers them that much.

As much as I'm tempted to do just that, I know it will just end up being a boomerang. As noted in the accepted answer, they won't look into it and see that the warning is spurious -- they'll just file an issue ticket. They won't look at the notice in the project documentation to the effect that the warning doesn't apply -- they'll just file an issue ticket.

And, since they most likely won't read the source code anyway, the few one-line comments sprinkled around the pointer casts won't raise any hackles, because they won't be seen.

So, note to self: realistically, there won't be a lot of eyeballs on the source code, so don't anticipate grief where it's unlikely to occur.

EDIT While I still think this is a completely tenable position, I had to change the accepted answer because someone actually solved the problem.

By defining the suggested macro directly in the queue implementation, the uncertainty is resolved at the source, and explained at the point of resolution.