Use an enum for Assets - Githubissues

coblox / bobtimus

an autobot that automatically handles swaps as Bob

Apache License 2.0

1 stars 1 forks source link

Use an enum for Assets #51

Closed D4nte closed 5 years ago

D4nte commented 5 years ago

New Asset enum
Refactor static rate configuration to make simpler
Starting the work to have a better interface

D4nte commented 5 years ago

@da-kami let's squash the WIP commit before merging :)

luckysori commented 5 years ago

I have removed the automagically-mergify-this label so that you can look at my non-blocking comments! Feel free to merge without changes though :smile:

D4nte commented 5 years ago

Bear in mind that none of my comments are blocking, but I did find some of the code I looked at hard to understand. I don't know how much we care about code clarity in Bobtimus though.

The main question is: is it harder or easier to understand from before? This is hard anyway, so it's not like we made something easy to conceive into hard code. It's a complex problem and we have less-complex code now.

I welcome suggestions that'd make the code easier to reason about....

D4nte commented 5 years ago

I'll trust you that the logic is on point. The wording could maybe be improved :)

@thomaseizinger please provide suggestions on how it can be improved. This is the best we could come up with @da-kami.