Re:Reducing technical debt by valuing comments as much as code
by brian_cunnie on Wednesday February 01, 2023 @09:40AM
(#34611050)
> 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!
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.
[0]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429