Olin Lathrop wrote: > Self documenting code where the author thinks its a replacement for > comments > is usually worse than terse names with profuse comments. The right answer > is of course to do both. Names you think at the time tell you everything > you need to know will leave things out. Let me say right off the bat, that I am not suggesting not using comments at all. The problem I see is that well-meaning programmers use comments to try to make up for the shortcomings of their code. I think they should practice writing self-documenting code, because it has several important advantages over comments: - Function and variable names rarely get out of date - Good names eliminate the need for line-by-line comments - You cannot "forget" or "put off until later" naming a function or a variable - It helps the programmer maintain code cohesion, by focusing their attention on the purpose of the function This last point is very significant, IMHO. Comments are passive -- you use them to describe what a function does, often after you've already written it. A well chosen name actively shapes the implementation of a function. If a piece of code you're trying to put into the function does not agree with the name, you know you should either change the name, or (which happens more often) put the code into another function. > Good comments will complement well chosen names. If you really believe > the > name is clear and tells you what it's about, use the comment to describe > the > next level up purpose. I believe that the real purpose of comments is to provide the context, to explain *why* something is being done (never *what* is being done). If you follow the principles of tight cohesion, and keep the functions short (20 lines or less, is my rule of thumb) you can name the function in a way that describes exactly what it does. > Even if the subroutine name tells me you are getting > the forward voltage, tell me why you are getting it here and now, for > example. In fact, the real purpose of good naming is to allow the comments > to provide this next level up information while leaving less confusion > about > the details. This goes agains the principle of loose coupling. The function should only be responsible for itself. It has no business knowing anything about the function(s) that are calling it, or what they're doing with the return value. > A well chosen name doesn't mean rediculously verbose either. Horizontal > space is precious, in part because it eats into how much you get to > explain > in the end of line comment. If you want to reference a symbol in a > comment > additionally, a long name eats into characters that might be better spent > telling something else about the symbol or why it is being used here. That's actually an excellent argument for not commenting every line. Use the space you would otherwise waste on comments, to create a descriptive function name. Make it as long as necessary. If the name is too long, it will tell you that the function is doing too many things, and needs to be broken down into smaller, more cohesive functions. > While it's good to have symbol names give some idea what they're about, > you > should think of characters spent as being precious and use them wisely. Why?! Characters in symbol names are *free*. Your suggestion goes against the recommendations of leading programmers, and common sense. [snip] > This is a good example of a useless verb and a missing comment. This > function takes no arguments and returns a value. In the code: > >> forwardVoltage = GetForwardVoltage(); > > It would be pretty obvious this function is getting something. Make the function name more vague -- to save three letters? Functions are verbs, which act on nouns (variables). Therefore, a function name must have a verb in it, to help improve understanding. Moreover, a lot of times you have no other option but to use a "Get" and a "Set". For example: void SetOvenTemperature(int temperature); int GetOvenTemperature(); > The bad part > about these two lines is that the comment was omitted. That is a wasted > opportunity to explain the next level up what the function is about or > what > the forward voltage is or why you'd want to get it right there. So, change the function name to say exactly what it does. Put the comment inside the function name, and you won't have to remember to copy/paste the comment everywhere you use it. >> Or like this: >> >> if (GetForwardVoltage() > 100) >> DoSomething(); > > Better would be: > > if (ForwardVoltage() > 100) { //finally reached minimum level ? > or > > if (ForwardVoltage() > 100) { //above safe operating limit ? Unfortunately, neither comment makes sense in the context of the application: http://www.jasonhsu.com/swrwatt-c.txt If they did, then I would suggest something like: if ( FinallyReachedMinimumLevel() ) LightMinimumLevelLed() or if ( IsAboveSafeOperatingLimit(ForwardVoltage) ) SoundCriticalAlarm(); > Note how the comment makes a huge difference in describing the purpose of > the statement. Another great reason why you shouldn't use them: you don't know whether the comment truthfully describes what actually happens. Put the comment in the name instead. > As a separate nit pick, I wouldn't want to see "100" hard coded like that. > That should be a symbolic value defined at the top or in a master include > file. Agreed. "Magic numbers" should be avoided. > So the two statements really should be: > > if (ForwardVoltage() > MinValid) { //finally reached minimum level ? > > if (ForwardVoltage() > MaxSafe) { //above safe operating limit ? Again, the usefulness of these examples is limited because they don't reflect what actually happens in Jason's code. It's not "Minimum Level" or "Safe Operating Limit", it's just a value that Jason is using to determine whether to light a particular LED in the LED bar. IMHO, comments have three uses: - Provide context - Explain why something is being done in an unexpected way - Seve as crutches when I'm too lazy to come up with a good name Only the first two uses are legitimate. I still use the third one occasionally, but I always try to go back and refactor the code to make the comments unnecessary. To me, comments are red flags that mark spots in my code that need more work. Best regards, Vitaliy -- http://www.piclist.com PIC/SX FAQ & list archive View/change your membership options at http://mailman.mit.edu/mailman/listinfo/piclist