RE: Update for steemengine - token transfer and market operation added
It's always great to see you supporting all kinds of projects with Python packages. I've personally not really checked out STEEM ENGINE, but I saw that a lot of people were very excited about it, so I'm curious to see where it goes. I'm also always very interested in checking out the code itself, as you are a great developer and I think other Python developers can learn a lot from you.
Saying this, there's still some feedback I'd like to give:
- There are a lot of unused imports floating around multiple files. For example, in
rpc.py
you never usetime
,threading
etc. - Using
{}
or[]
as a default value isn't best practice: "this 'default' array gets created as a persistent object, and every invocation ofmy_method
that doesn't specify an extras param will be using that same list object—any changes to it will persist and be carried to every other invocation!". - In
wallet.py
in the functionchange_account()
you don't actually use thecheck_account
variable - not sure if you just missed this or something is wrong there. - Great docstrings which make it very clear what the functions do. Adding the examples is a nice touch as well!
- Imo some variable names like
h
andc
could be improved (and some variables are camelCase, while others snake_case), but other than that everything is very readable. - Would recommend using
pytest
overunittest
as it's imo better.
Good update with a lot of work. It's always a pleasure seeing your posts, so I can't wait to see future updates!
Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.
To view those questions and the relevant answers related to your post, click here.
Need help? Chat with us on Discord.
$rewarding 30% 13min
Thanks for the review.
I check if the given account is an existing steem user name in
change_account()
. I do not need the account object, so it is not further used. I think I can just remove the left side (check_account
).I will use your suggestions for my next update.
Thank you for your review, @amosbastian! Keep up the good work!