|
"My code doesn't need comments because it is self documenting, all methods are small and have single functionality, and any business documentation should be provided by the specification and not the code."
Discuss.
|
|
|
|
|
Must be nice where you work.
|
|
|
|
|
I wasn't clear - this was a quote from a cow-worker. My comments, unfortunately, were not KSS so I couldn't post them here.
|
|
|
|
|
|
_Maxxx_ wrote: cow-worker Someone who works the cow?
|
|
|
|
|
"Go forth into the source" - Neal Morse
|
|
|
|
|
|
If you don't need comments you're not doing it right, sunshine.
|
|
|
|
|
Simples:
double CalculateTax(double fine) {
var notAComment1 = "Calculate the tax based on the rate from the database.";
return fine * GetTaxRate();
}
Seriously, though, I like comments, even for private members. And I like everything (except for constants and member variables, which typically have a corresponding property) to have a comment. I mostly read the comments rather than the code when I'm quickly navigating through code. I don't want a funky looking uncommented section to slow me down. I also write XML comments so intellisense helps me avoid having to go to the definition to find more info.
|
|
|
|
|
I think comments are worth their weight so ling as they are written well - describing the business reasons not the technology (unless the tech is crafty, unusual or complex)
when I sit down to write a method, I start by calling it something
public double CalculateTax(double fine)
{
}
Then I comment it
Then I might write some test code just to get it building
public double CalculateTax(double fine)
{
return 34567.89;
}
Then I start to flesh out the method by way of comments
public double CalculateTax(double fine)
{
}
Then, finally, I write the code
public double CalculateTax(double fine)
{
double taxRate = GetTaxRate();
tax = taxRate * fine;
return tax ;
}
That way, I can remember where I was if I get interrupted, the comments aren't an afterthought, they are a part of the process and, if I get hit by the Programmer bus, someone else should be able to see what I was doing.
Obv. the example is small and trivial, but that's how I work and I fail to understand the 'don't need comments' brigade.
What I do hate is/...
return rate * amount;
which is simply a case of bad commenting in my book - it is not necessary to comment every step
|
|
|
|
|
_Maxxx_ wrote: which is simply a case of bad commenting in my book - it is not necessary to comment every step
True. The only time I write comments like that is out of aesthetic necessity. If there was a chunk of code above that line that had its own comment, but the comment didn't apply to that line, I might make a comment like that simply to maintain visual consistency.
|
|
|
|
|
I came across some code only a few minutes ago which was headed by a "Reset the totals".
the next three lines did as advertised.
the following six did all sorts of unrelated stuff; even a comment
// do unrelated stuff
would make it easier to read!
** Caveat. Mine is not the best code in the world, and I am not the 'best' programmer in the world. It's easy to criticise other's code and to say how you would have done it differently; what I objected to in the original case was the assumption that their code was so beautiful it didn't need comments, and that if info was required one should look at the spec (which , in the real world, doesn't actually exist!)
|
|
|
|
|
_Maxxx_ wrote: What I do hate is/...
return rate * amount;
which is simply a case of bad commenting in my book
I agree.
It's far too much typing, and excludes reuse of the function.
It should be:
return a * b;
At least, that's what I'm used to seeing.
I wanna be a eunuchs developer! Pass me a bread knife!
|
|
|
|
|
I so agree with that.
When I worked, I tried to impart the huge value of Comment First Development. It is always easier to describe what you want to do then it is to write the code. In the trivial cases it can be annoying, especially on accessor methods [properties to you C# boys], but even when you get to something as simple as your tax calc having the comments in place makes a great difference.
The second, and not to be ignored, advantage of CFD is that it is very easy to leave off the comment once you've finished the code.
Panic, Chaos, Destruction. My work here is done.
Drink. Get drunk. Fall over - P O'H
OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre
I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer
Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
|
|
|
|
|
I am more of an adherent to the "comments are bad" brigade, so will offer a counter view. To be more precise I agree more with the statement that "all comments are apologies [for not making the code self documenting]". I do like statements of intent, not implementation.
In the example given, I like the function comment, it states the intent and required preconditions.
///
/// Calculate the tax, taking into account the fine passed.
/// Requires that the tax rate is retrievable from the TaxService
///
Whereas the following two appear to me to be worthless and merely clutter up the code. I can tell the first line is getting the tax rate, by the call to the self documenting function, and I know the rest is calculating the value because it is obviously a calculation and it corresponds with what the statement of intent in the function documentation was.
// Get the tax rate using the appropriate service
double taxRate = GetTaxRate();
// calculate the fine
Comments like the above make code harder to read IMO just due to volume of text. More importantly they are often not updated perfectly when code is maintained, especially when scripted edits are performed; I have been misled in the past by reading the comments and the two not corresponding and this has cost me time, so I would prefer to just read the code and not be distracted. Also where they are a repeat of the code they fail the DRY principle.
Like yourself I do write comments during the process of implementation, if I want to sketch out some pseudo code in a comment then slowly turn it into code (just in case I win the lottery and someone else has to finish it off). The difference for me is that once I have finished the code, the comments will have been almost entirely replaced by the code.
|
|
|
|
|
M Towler wrote: Whereas the following two appear to me to be worthless and merely clutter up the
code. I can tell the first line is getting the tax rate, by the call to the self
documenting function, and I know the rest is calculating the value because it is
obviously a calculation and it corresponds with what the statement of intent in
the function documentation was.
// Get the tax rate using the appropriate service
double taxRate = GetTaxRate();
// calculate the fine
That comment adds meaning to the code - it tells me that the rate is obtained from a service as opposed to being determined within the function.
Your argument would have more vilidity if the function was renamed from GetTaxRate to GetTaxRateFromAppropriateService although even then I would expect some comment within the function to define what "Approprate" means
|
|
|
|
|
I feel that level of commenting or descriptive naming would be breaking encapsulation, by exposing the implementation of GetTaxRate. I will go further and say that it would be repeating some of the documentation of GetTaxRate, so fails DRY. This piece of code is simply doing a calculation, so it calls a function to get the rate. It does not really need to have knowledge of where the rate came from to do its task, so it is best not to give it this knowledge. Having minimal information in this function means it will not become out of date when the implementation of GetTaxRate changes, such as to add a local cache of the value obtained from the service, using a database of rates or some other alteration.
In summary, what I am really saying is that comments of this type raise the cost of maintenance, as each later change will have to find and change multiple comments in addition to the code itself, or else risk misleading future maintainers.
|
|
|
|
|
Agreed.
It's just my way of viewing this, but I think that simpler is better when writing (and later reading) code, so adding comments that just repeat what the code is already saying or expose some behavior that whoever is using it just don't need to know is failing this principle.
That's not to say that any comment is bad either, and I really liked the "comment first and them code and remove the comment" approach!
|
|
|
|
|
greldak wrote: That comment adds meaning to the code - it tells me that the rate is obtained from a service as opposed to being determined within the function.
Actually, that's a poor comment. It tells you about what the GetTaxRate() function is doing.. something this function has no control over nor should it care. What if tomorrow, GetTaxRate() were reimplemented to get a tax rate some other way? Now, at best, the comment here is misleading. At worst, if the remainder of this function depends on that tax rate having been looked up in some service, it will break.
So, since the source of the tax rate is unimportant to the code written here, the comment should really read:
// Get the tax rate
And how is that more helpful than the code itself:
double taxRate = GetTaxRate();
Like you suggested, if its important that the tax rate is fetched from a service, then the code should read:
double taxRate = GetTaxRateFromAppropriateService();
Which still probably doesn't need a comment here because what an "appropriate" service is, is defined within GetTaxRateFromAppropriateService().
We can program with only 1's, but if all you've got are zeros, you've got nothing.
|
|
|
|
|
I agree with this sentiment.
Further, I feel compelled to comment when there is a pitfall to avoid.
///
/// Calculate the tax, taking into account the fine passed.
/// Requires that the tax rate is retrievable from the TaxService
///
/// Note that we subscribe to a service (CheapTax) that may return an
/// undocumented -9999.0 as an error value. Be sure to check for this
/// value before continuing.
///
double taxRate = GetTaxRate();
|
|
|
|
|
Yay hay!!! I KNEW I wasn't the only one out there!
I'll sometimes throw in comments like //Use of array[1] is deliberate and correct or whatever when there's any risk that someone may refactor / fix a "bug" without really thinking it through - apparently there are developers like that
|
|
|
|
|
Truly. When I am doing somethings that either is hard to figure out, not intuitive, or appears at first to be solutions in search of refactoring (but in reality is not), I like a comment to remind me why I did it the way I did, and to let others know there is a reason for the funky implementation.
|
|
|
|
|
Great comment! I do it pretty much the same way. While I am thinking what the comment should say to me in 5 years, I usually get good insight what the code-approach should be to work best. When I work on my 5 year old code, I many times wish I would have thought the same way 5 years ago.
|
|
|
|
|
Back in the day this used to be called pseudo code and was the only way we were allowed to get through the design process. If we couldn't describe what we wanted to do in comments how could we expect to code it?
|
|
|
|
|
CalculateTax() may be a bad name - it may not be specific enough. It's not that far away from DoSomething() . OTOH, provided it's a private method, used in a context where its meaning is obvious, why would it still need comments, especially if its body reads like return amount * db.GetTaxRate() ? Oh, you have GetTaxRate() in the same class, and it's accessing the database directly? That's a clear case poor encapsulation, where two classes, one coupled to the persistence layer and one strongly rooted in the business logic are shoved into a single one, and which need to be separated.
I too profoundly dislike comments. I think comments are a code stench (M. Fowler calls them smells, but I think "stench" is more descriptive). Every time I see comments in code I find that they are either useless (the code is quite understandable even without them) or are used to mask other problems (the code absolutely needs some refactoring).
Properly maintaining comments anywhere except in libraries which you develop for use across several projects, which you may plan to distribute without source code, is IMO waste, in terms of agile methods, and should therefore be eliminated.
Comments are also the source of many stupid problems. Since the compiler doesn't check comments, they tend to quickly become buggy, this being the case especially in heavily commented code. Having been bitten by this many times, I tend to disregard comments, and as such heavily commented code is annoying for me, since I have to skip large portions of prose to get to the parts which actually matter - and which I can actually trust.
Also, if you read a C++ header file or a Java or C# class file, and there's maybe 10 lines of comments between any two method or member declarations, you might get a very good understanding of the individual members, but loose the overall understanding of the class you're looking at. This increases the chances of introducing changes which break the overall architectural consistency, or even rightout introduce subtle conceptional bugs which are hard to diagnose.
Really, if your code needs commenting, take a better look at it, and think how you can refactor it into smaller, more specialized functions, with better names. Your code should become easier to read without comments if you do such refactorings. And there's a non-documentation-related gain too: you'll notice that code duplication goes down and your code becomes more compact.
In his book "The art of Unix programming" Eric S. Raymond states that he two most important attributes of user interfaces are discoverability (you can easily find out how to use the UI by playing with it) and transparency (you can easily build a correct mental model of the underlying application's workings by analyzing its user interface). IMO that's the case not just with user interfaces, it's the case with any product created by humans, including source code. If your code needs comments, it lacks at least one of those two properties. If something (really, anything) doesn't have any of those properties, people won't want to use it.
|
|
|
|
|