ToohrVyk:
Excellent! Just what I was looking for, a good practical example. But a question still lingers. Would you scroll through 100,000 lines of code looking for your pattern or use the search feature on a unique header? To find the 3 in your example, the highlighting works on a few 1000 characters when the user is too lazy to Ctrl + F search for 3. Great for visual que inside something small like a function.

Ezbez:
Comments as a scratch pad. Not a bad use. Going to add to exceptions!

[Edited by - KodeWarrior on March 21, 2007 7:28:43 PM]

Quote:Original post by Structural

Quote:Original post by Adam Hamilton
Imagine this: You need to add a certain feature in say OpenOffice. Now, wouldn't you prefer that the OpenOffice programmers commented their code so you would know where you need to be to add that certain feature.

I'd rather have them write readable code.
A common pitfall I often see (and also used to do) is to overgeneralize, and then having to comment how to use the generalism.

For example,
void SwitchMode(int mode);

would be much more readable if it was:
void SwitchToEdittingMode();
void SwitchToBrowsingMode();

1) SwitchMode() is not very descriptive of the method's functionality
2) obscure parameter, though that could be improved by using an enum


http://c2.com/xp/CodeSmell.html

Note that comments are listed as code deodorant.

That wasn't the point of what I was trying to say. Are you saying that you would be happy to add a feature to some software that has absolutely no comments in exchange for very specific function names and variables? Interseting...

Quote:Original post by ApochPiQ
The name of the function is AccumulateStrafeVelocity. I'm sure we'll get a lot of symbol collisions for that one.

I think the implication is that you might at some point want to have a function which returns the same vector in a different coordinate system.

AccumulateStrafeVelocityInWorldCoordinates, perhaps? [smile]

A fairly robust solution would be to specify the coordinate system to be used in each call to AccumulateStrafeVelocity.

A totally robust but less efficient solution would be for a vector to identify the coordinate system it uses, so that it would always be okay to write AccumulateStrafeVelocity(...) + GetSomeLocalVector(). Where efficiency mattered you'd have to get a "raw" vector without an associated coordinate system.

Quote:Original post by KodeWarrior

Quote:
What if I implement a huge project with BubbleSort scattered all over the place, then suddenly learn that bubble sort is a pretty bad algorithm, and go to replace it with QuickSort? I've leaked an abstraction and added a refactoring job which I never should have had to worry about.

Tell that to the people who wrote qsort() and bsearch().

qsort(), despite the name, might not be QuickSort. On many systems, it is actually a merge sort. Where it is a QuickSort, it is often a modified QuickSort which can cope with mostly-sorted input. Additionally, it will almost always use a different algorithm when the list to be sorted has fewer than twenty or so elements.

bsearch(), similarly, might not be a binary search. Any algorithm which takes advantage of the fact that the array is already sorted could be used. The binary search is the best known such algorithm. It's possible that bsearch() might actually use an interpolation search on large arrays. On arrays with just a few elements, linear search can be faster. A concurrent computer could partition the array between CPUs and search in parallel. In the future, a quantum computer might use Grover's algorithm to speed the search up even further.

Quote:
How do you propose to fix the problem without refactoring?

Modify Sort(). Compile. Run.

Quote:
I would hope sorting of strings, small lists, large lists, lists with small range would all be calling different sort functions if you care about efficiency.

You would, huh?

Well, Sort can trivially be overloaded to prefix-sort strings, and it can decide for itself whether a list is small or large.

Sort can be overloaded to perform a pigeonhole sort of arrays of chars or shorts. It can probably check an array of ints for pigeonholability quickly enough that the time spent would be worth it, or it might just use a bucket sort.

A possible weakness of the STL is that there's no direct support for sorting by key: if you want to do so you must usually override the comparator, which will make it impossible for std::sort to use a linear-sort. An alternative to using a custom comparator would be to use a Schwartzian transform.

Quote:Original post by KodeWarrior
States the obvious, therefore redundant.
//initialize variables
x = 10;

yes. you are stating the obvious, it's therefore redundant ;) The golden rule is :

comment why you are doing something, not how you are doing it

and then comments are a wondeful aid to the other programmers on your team (and you in 6months time when re-visitng old code).

Quote:Consider writing a standards document once, no need to copy and paste standards into every file. Should be obvious in language where external files are included, globals are declared, etc. Don't state the obvious.

I do :

//------------------------------------------------------------------  Globals

and do the same for all classes defined in a header. No, it is not bad. The text appears on the right hand side of all code, which makes it trivial to scroll up and down to find the correct class you are looking for. It saves me time and therefore i do it. That is something called nice code layout, and it is certainly not bad (I use Visual Assist auto-text templates for pretty much all code constructs so that they all appear uniform - the comments are part of that). But then again, that's my style and i like it. You might have another style, but to say it's bad is your opinion.

Quote:Original post by KodeWarrior
Alerts user to all assumptions made. Problem is with code, comments don't "fix" the problem. Who reads 'em anyway? Use ASSERT, CASSERT (compile time assert), or even make impossible with refactoring.

This is just good programming practice and has nothing to do with comments.

Quote:Original post by KodeWarrior
or return an error code
enum Errors{ERROR_OK, MUST_BE_POSITIVE};
Errors f(int x){}

No. don't return an error code. assert. no-one checks error codes.... (except wierdo's like me).

Quote:Original post by KodeWarrior
Clarifies obscure code. Problem is with code, comments don't "fix" the problem. Who reads 'em anyway?

As an experienced coder i beg to differ on this. When you have 2 days left on your big deadline, you don't go around doing large scale refactor's to "fix" the problem - that will introduce more bugs, and chances are your deadline will slip. You do the smallest amount of code changes possible to ensure that all of your current tests pass. ie, you hack it!

If you are worth your salt, you will put a decent comment in there to say, "look, i hacked this because of X, and due to deadline y. This is what the hack does and why....". Again, if you are organised, you put the refactor down as a high priority task when you enter the next dev cycle. Worst case scenario, there are higher priority tasks in your schedule and you end up fixing it in a few months time - by which time you'll be bloody glad of that commenting i can assure you!

Quote:Original post by KodeWarrior
Use pragma
#pragma message("TODO: Add something here")

or....

int TODO; // Add Something here...

which will be flagged as a warning for an unused var at compile time on even those compilers that do not support #pragma message.

Quote:Original post by KodeWarriorI'm interested in any exceptions to the following rules you can come up with.

I develop middleware for the games industry... I can think of plenty ;)

Documenting != Commenting
funcs in headers like

void doSomething(int,float);

should be written with their args, ie :

void doSomething(int flags_for_blah,float some_verbose_description);

this will give people using your code the ability see the args required and a description via intellisense. If the arg descriptions are not clear what they mean, they can consult the doxygen generated docs on the func, generated from the comments....



Whilst it might seem a bit over the top, bear in mind that the more time we (the developers) are roped into client support, the less time we have to actually develop. We therefore take a *lot* of time providing verbose commenting and documentation to ensure that our clients are very aware of how to use the code drop. Every e-mail they don't need to send us, is an e-mail we don't have to reply to. The cost of documenting a function once is always less than replying to 30 different developers asking the same question.

Cross platform funkyness

The land of cross platform development is much fun, especially when you are under pretty strict NDA's from Ninty, Sony and MS to not release any code from their respective SDK's (or any mild info about their SDK's) to anyone that does not have a valid dev license. So, If client X is developing on 360 only, we cannot ship anything within the source code that uses something silly like :

#ifdef PS3  // some PS3 funcs... #else#endif

This leads to situations where the source code does something that *seems* redundant (and probably is on the target platform - though it won't have a runtime effect at all.... ). Without a comment, some of these things may make us look like complete idiots....

#define BLAH(X) ;// required to maintain cross platform compatibilityfor(int i=0;i!=22312;++i)  BLAH(i);

But again, we aren't allowed to hint at PS3, 360 or Wii in those comments..

Asserts

Whilst above you suggested :

ASSERT( (condition) && "Some Description" );

I prefer :

// Some very verbose description, and info about how to solve the problem// you have just encountered that made you (the 3rd party developer)// assert here.... // ASSERT( (condition) && "Some Brief Description" );

Again, this is code documentation, not commenting....



Anyhow, to re-iterate what i said at the beginning, comments are a fantastic tool to be used as often as possible, so long as they follow the following rule...

comment why you are doing something, not how you are doing it

Here's a good source of debatable comments:

http://www.gelato.unsw.edu.au/lxr/source/arch/i386/kernel/apic.c (et al),

Comments like "Pound the ESR really hard over the head with a big hammer"

Now, you might consider that comments like this are in some way warranted, for this being the Linux Kernal source, it needs to deal with a bunch of indeterministic hardware. Or maybe you say the names of the hardware registers are not quite meaningful enough to be self-explanatory.

Well, unfortunately, that's the way it is with a lot of software. Not everyone works in teams of one, or even seven, or twenty. Not everyone gets to write their own game engine with a group of close friends. Some people have to work with what are essentially huge indeterministic systems that have badly named interfaces. The interactions you perform with these systems often need comments.

Even on small teams, the sad truth is that the twin furies of deadlines and inertia tend to fold your code into something slightly fishy. Comments at that point might be a symptom, a palliative, even a crutch, but that does not mean you go cold turkey. Comments are not in and of themselves a bad thing. You don't want to get rid of comments. You want to get rid of the need for comments.

Quote:Original post by ApochPiQ
The whole coordinate-system-tracking concept is too off-topic so I'll leave it alone.

I disagree.

I think the specific mechanics of self-describing vectors are off-topic, but whether it is better to have self-describing vectors or vectors described by a comment is not.

Quote:Original post by ApochPiQ
All I wanted to illustrate with that case was that occasionally it makes sense to document function returns, because sometimes you get a weird edge case where the return is slightly inconsistent with what one may expect, but there's no point making major code changes to "accommodate" this difference.

That's not what you illustrated, though. You illustrated that occasionally it makes sense to you to document a surprising feature rather than making major code changes to make the feature unsurprising. Other people may legitimately have a difference of opinion; that's why this thread exists at all.

Whether it would be better to use a comment depends upon many variables, such as how much code would have to be modified to accommodate self-describing vectors, what the chances of coordinate-space confusion occurring elsewhere in the code are, or whether the performance characteristics of self-describing vectors would be acceptable.

These variables are not the same for every project.