Hacker News new | ask | show | jobs
by komon 1743 days ago
In defense of the first comment, when first approaching a body of code like this, it's not always going to be clear what returning None is going to mean in any given method.

Also, one thing I think doesn't get mentioned enough is the use of comments as anchors for text search. Having "disable limiting" expressed in English that way makes it easier for newcomers to explore the codebase.

2 comments

Good point. Also the phrases in the initial comment "token bucket capacity" and "refill time" may serve as hints and can be used as entry points to searching external documentation.
I agree, though I think it would be even better if variables were named "token bucket capacity" and "refill time" in the first place.
It’s hard to judge out of context. IMO None meaning “disable limiting” should be a docstring/comment at the method level, describing what its return values mean, not inline next to this code.