Proper units (Wei, Tokens, Ratio)

[phraktle]

No description provided.

[szerintedmi]

Great one in general! A few semantic suggestions/questions

1. instead of Tokens.parse(string) wouldn't be better:
   Tokens(string|number|BN|Tokens)
   It's the defacto constructor standard in bn.js and BigNumber (although it doesn't mean it's the best way.. )

2. Percent is not really percent.. Should it be Ppm ?

3. Tokens.of(number), Percent.of(number) and Wei.of(number) :

   • .of reads a bit weird for me. not sure what would be better. Idea:. Wei.fromEth, Ppm.fromPercent and Tokens.fromDecimal ?
   • allow .of(number | string )?

[phraktle]

1. instead of Tokens.parse(string) wouldn't be better:
   Tokens(string|number|BN|Tokens)
   It's the defacto constructor standard in bn.js and BigNumber (although it doesn't mean it's the best way.. )

I don't consider this a good practice: if you meant to parse a string, a type error should be thrown if you pass in anything else. Also, other parameters may be added later, which may not make sense for other types (e.g. base).

2. Percent is not really percent.. Should it be Ppm ?

Started off with PPM, but Percent seemed better for fluent readability and semantics (it's a dimensionless ratio and would still make sense if we happened to use 5 or 7 digits of precision). Potential alternatives are Fraction, Ratio or Rate. But I'm okay with PPM as well.

(Btw, I was considering Ethers instead of Wei. What do you think?)

3. Tokens.of(number), Percent.of(number) and Wei.of(number) :

   • .of reads a bit weird for me. not sure what would be better. Idea:. Wei.fromEth, Ppm.fromPercent and Tokens.fromDecimal ?

A more verbose form would make readability worse in test cases (plenty of constants there). Foo.from(..) might be okay, if you find that more pleasant to read.

• allow .of(number | string )?

same as above, with even more confusion (number is treated as a decimal. should a string be interpreted the same way here?)

[rszaloki]

Please fix ts lint errors!

[phraktle]

@rszaloki is there a build target to run the linter? Or if it’s some other process, could you please add it to the README?

[phraktle]

@szerintedmi Percent renamed to Ratio.

@rszaloki Fixed some linter warnings. Some others I happen to disagree with :)