ghoul-sol / treasure-staking

10 stars 1 forks source link

"Boost" word is used inconsistently #33

Open fulldecent opened 2 years ago

fulldecent commented 2 years ago

If you'll just bare with me for a moment, this next paragraph explains lunch I recently had:

Down the street is a restaurant where I get khachapuri. The menu lists the price as USD 12. But the menu also says all prices are "temporarily" increased by 25%. Final sale price is USD 15 which is 125% of the original price. (Plus tax, service fee and tip.)

Some of the implementation such as getExtractorsTotalBoost uses "boost" to describe the 25% in the story above, such as getExtractorsTotalBoost. Other parts of the implementation such as getHarvesterBoost use "boost" to describe the 125% in the story. Also "boost factor" is used in LegionStakingRules.getHarvesterBoost and in the documentation and they represent the 25% and the 125%, can you guess which is which?

Using the same word to refer to two incompatible things reduces clarity and creates a risk of mistakes in the implementation, now or in the future.

Recommendations:

  1. Select and consistently use two different words to describe the 25% and the 125%, perhaps (boost addend and boost) and clearly explain this concept in external documentation (including NatSpec).
  2. Consider to refactor (i.e. add or subtract ONE) so that all functions use one or the other for its inputs and outputs.
  3. Try khachapuri if you haven't already, it's delicious.