Writing Well-Documented Code – Learn from Examples
I had a lot of trouble reading that code. I kept getting distracted by the comments and missed the code. The code seems pretty self-explanatory. I have some changes I would make, but they really depend on performance vs depth (modulo the burst vs budget bug).<p><pre><code>  &#x2F;&#x2F; Hit the bucket bottom, let&#x27;s auto-replenish and try again.
  self.auto_replenish();
</code></pre>
Is ugly. Why repeat in English exactly what the code says? I don&#x27;t even know what language this is, but I know it&#x27;s replenishing the tokens in the bucket.<p>All of the comments in reduce are redundant, this is what I would write at the top, so that anyone using the method doesn&#x27;t have to page through to figure out what it does, it also makes the &quot;smells&quot; more obvious:<p><pre><code>  &#x2F;&#x2F; reduce: attempt to consume tokens from bucket, 
  &#x2F;&#x2F; starting with one_time_burst, overflowing to 
  &#x2F;&#x2F; budget. 
  &#x2F;&#x2F; (keep comment only if _expensive_):Performs
  &#x2F;&#x2F; auto-replenish when
  &#x2F;&#x2F; burst is emptied, and budget has insufficient
  &#x2F;&#x2F; tokens.
  &#x2F;&#x2F; Asserts size, but only on replenishment.
  &#x2F;&#x2F; On OverConsumption, entire bucket is emptied. 
  &#x2F;&#x2F; On Failure, only one_time_burst is emptied.
  &#x2F;&#x2F; In all cases, tokens is reduced by amount 
  &#x2F;&#x2F; fulfilled.
</code></pre>
I don&#x27;t understand why OverConsumption is different to Failure. Both will result in throttling by the caller. The reason for the difference should be documented.<p>I&#x27;m also curious why burst is consumed, then budget. I would expect _budget_ to be consumed first (with refill) with overflow into burst? My expectation is for burst and budget to have different refill schedules in auto_replenish, so using burst first would result in more failures by missing refill opportunities.<p>I&#x27;m also curious why the size assertion is only on requests that result in refills. Is it guaranteed that size = max(one_time_burst + budget)? Why care about the difference? Is hogging the tokens &quot;bad&quot;?<p>Then, I&#x27;d work to remove the need for those comments - particularly the need to explain what happens on error - which should be &quot;nothing&quot;, or that the bucket is emptied of whatever is available, not something in between.<p>Finally I&#x27;d remove the mutation of tokens, returning either unfulfilled or fulfilled in the return value.