> I very rarely see a comment which is genuinely both well placed and useful
Your experience is different than mine: I write software which interacts with cloud (Amazon AWS, Google Compute Platform, VMware vSphere), and a well-placed comment describing the quirks of whatever platform I'm working with helps immeasurably, e.g.
> VMware NSX will throttle us with an HTTP Status 429 "Too Many Requests" if > 100 API calls/second. In that case, we back off and retry rather than fail.
Agreed. I’ve found over the years it is helpful to prefix comments like that with something like “HACK: “ as it makes it clear you are working around a limitation and not adding a feature - plus it stands out when browsing the code.
My bad for not being clear, I was really talking about the comment saying 429 means “too many requests”. By all means comment to say that service X has a rate limit of N req/min but a comment saying 429 means too many requests is exactly one of those things that’s better expressed in an enum.
301; 302; 401; 429; 504; I shouldn’t need comments to tell me what they are, nor should it be expected that anyone touching the code knows them off the top of their head.
Unless the API itself is poorly designed, 429 _means_ "you have reached a rate limit". That might not be so self explanatory for someone who isn't familiar with HTTP error codes, but that person maybe shouldn't be attempting to read a codebase which deals with making HTTP requests in the first place (e.g. my keyboard firmware doesn't have a comment explaining how a keyboard matrix works). It may be worth noting what the rate limit is somewhere, but likewise, I'm not sure how useful that is. If the rate limits change then your comment is now incorrect, but the code to handle backing off due to a rate limit should probably still stay, and should work independently of what the actual rate limit is. If someone questions why that code was included (first off, not sure who would question what seems like a perfectly sensible design decision) then that person can refer to the VCS blame (e.g. git blame) logs to find what _should_ be a detailed commit message explaining all of the 'in the moment' reasoning for why and how the code was written.
I don't mean to imply that your code is poorly written as I clearly can't see it and maybe it's a lot more complex than I am imagining it, but I don't really understand how your code was written such that it became unclear enough what code handling a 429 was doing such that you felt you needed a comment to explain it, but I can envision a codebase which handled rate limiting where comments weren't needed to explain the reasoning behind it.
I think the best way to think about it is this: What information is this comment conveying?
- "HTTP Status 429" means "Too Many Requests" - You can put this information in a comment every time you refer to 429, or you can simply use an enum.
- "429 Too Many Requests" means "a rate limit has been hit" - You can put this information in a comment every time you refer to the enum, or maybe it's best placed on a website like MDN[0] where it can be surrounded with far more detail and where the information has a much smaller chance of becoming outdated.
- The rate limit is 100 API calls per second - You can put this information in a comment every time you talk to this particular API, or it's best placed in a document which describes the API itself such that if you ever need to update it, you can update it once. Maybe you can demand that the vendor of the API provides and maintains this document as part of whatever contract you have with them. If necessary, the module of your code which deals with interacting with this API may benefit from a reference to this document.
- That the code should back off and re-try instead of failing - Okay, now we're getting to the core of something important. And I think the reality is more complex than the comment even lets on. Does the code re-try indefinitely? Does it have an option to disable retries in some contexts? I'd assume most likely a no for the first question and possibly a yes for the second. In that case, the names of variables within the code the name of the function, and the names of the parameters of the function should probably be enough to convey this meaning entirely.
That being said, I think that as far as comments go, there is still a benefit of having short descriptive comments for WHAT a function does for every function in a codebase. I've found that they're much more useful (as proper editor tooling can show you them whenever you make use of a function) and by being short and ONLY describing the _what_ and not the _why_ or _how_, the comments have a better survival rate and encourage people to avoid trying to make individual functions too complex.
In this case, your function may benefit from a comment such as:
// Request foo from bar, optionally backing off and re-trying <retries> times
The function signature itself might look something like:
foo(..., retries: Optional[int]) -> ...
Or alternatively you could do something similar with an explicit timeout instead.
The only missing piece I can think of is that if you have some form of back-off you probably need some initial delay before retrying, in this case this should probably be made a constant and this constant (without making any explicit references to its value or whatever value it is based on) may benefit from some reference to the method by which it was chosen. If you do intend to make it depend on the actual rate limit (e.g. 100 requests per second) then that should itself be a constant. But I think generally code like this should be avoided due to its fragility.
In any case, it is again difficult to give too specific of a recommendation given that I haven't seen the code but I don't think the comment you gave is a particularly good example of something I would personally ever recommend or want to see in the middle of a block of code.
> then that person can refer to the VCS blame (e.g. git blame) logs to find what _should_ be a detailed commit message explaining all of the 'in the moment' reasoning for why and how the code was written
Agreed, they "_should_" write the reason why, but many committers unfortunately write the "what" rather than the "why".
I like comments in code, and they're the first thing I read when, say, browsing the kernel or the Golang standard libraries/packages. And I appreciate that you don't find them as useful--more power to you!
People have different ideas of value. I see a method that sounds like that and a 429 and I have a pretty good idea of what is going on without someone taking up more lines to describe the method than execute it.
> I see a method that sounds like that and a 429 and I have a pretty good idea of what is going on
Yes, you're a seasoned HTTP developer. The comment isn't meant for someone like you. It's meant for the other developers on a shared codebase who may not know much beyond the standard HTTP status codes (200, 400, 403, 404, 503).
> taking up more lines to describe the method than execute it
It's a one-line comment. the Ruby code with the exponential backoff, eight lines. It does not take up more lines than the method.
You split developers in to two categories, but how do you know those are correct categories? What if there's another type of engineer that only knows about 200 and 400?
My approach is that conventional knowledge doesn't need to be documented in comments. Attempts at guessing how much conventional knowledge someone is lacking are futile. Those comments are similar to:
int a = 4 // set a equal to four
If someone doesn't know what 429 is, they should get confused, do their own research, and learn. Comments should not be turned into educational material about open standards, they should be about your business. There are much better places to learn about standards.
Your experience is different than mine: I write software which interacts with cloud (Amazon AWS, Google Compute Platform, VMware vSphere), and a well-placed comment describing the quirks of whatever platform I'm working with helps immeasurably, e.g.
> VMware NSX will throttle us with an HTTP Status 429 "Too Many Requests" if > 100 API calls/second. In that case, we back off and retry rather than fail.