search

probstj / ccGains

Python package for calculating cryptocurrency trading profits and creating capital gains reports
GNU Lesser General Public License v3.0
49 stars 13 forks source link

Refactor CurrencyRelations for readability #21

Closed anson-vandoren closed 5 years ago

anson-vandoren commented 5 years ago

I spent a while trying to figure out exactly how you were generating the 'recipes' for CurrencyRelation objects, and ended up with a pretty thorough refactor as I was working my way through it.

I think it reads better, and allows for a bit easier testing in the end. There's no change in functionality, and all of the original tests pass in both cases. I added a few more tests for some of the specific changes I made as well.

It clocks in at ~170 extra LOC, but operation time should be almost identical (fractionally slower because of some function overhead). The new classes are all subclasses of NamedTuple, and are just as lightweight as plain tuples. In my opinion, the extra readability is worth it, but up to you, and happy to hear any feedback.

I did use a different nomenclature for currencies in a pair than you've used elsewhere: base/quote currencies instead of from/to currencies. To me, this is less confusing and aligns better with standard financial terminology, but if it's a sticking point I can change it back relatively easily.

Regards, Anson

probstj commented 5 years ago

Very nice, thank you very much!