CyberPunkMetalHead / Binance-volatility-trading-bot

This is a fully functioning Binance trading bot that measures the volatility of every coin on Binance and places trades with the highest gaining coins If you like this project consider donating though the Brave browser to allow me to continuously improve the script.
MIT License
3.41k stars 777 forks source link

transform to modular design #92

Closed CyberPunkMetalHead closed 3 years ago

Davidobot commented 3 years ago

"modular design" =/= factoring out every function into a different python file

I think this makes the code significantly less readable, especially given the very linear nature of some of the function calls, eg buy -> convert_volume -> wait_for_price -> get_price

If I didn't know the codebase then jumping through all these hoops of looking into different files would make me rather quite confused.

If I were to refactor:

O-Mutt commented 3 years ago

I'm gonna echo a little bit from ☝️

This is less of a modular design and more of a function split.

I would love to see a portfolio.py that handles all the CRUD of the portfolio, binance_service that handles interactions (and error handling) of external calls to the service. And break it down into responsibility rather than files.

While this "split" may help PRs a little easier to handle I don't think it moves us toward the goal of building a more robust, understandable, and extensible codebase

getsec commented 3 years ago

@O-Mutt and @Davidobot are right. Im gonna take a stab at this.

CyberPunkMetalHead commented 3 years ago

Fair enough. In that case I will close this request and merge @Davidobot ’s PR next. My only comment on your PR is that we could use a bit more logging when coins are not being bought, I thought the code was broken when it wouldn’t print anything for minutes at a time.

CyberPunkMetalHead commented 3 years ago

@getsec might wanna base it off David’s pr to avoid conflicts

getsec commented 3 years ago

Fair enough. In that case I will close this request and merge @Davidobot ’s PR next. My only comment on your PR is that we could use a bit more logging when coins are not being bought, I thought the code was broken when it wouldn’t print anything for minutes at a time.

I think this is good. Once his PR is out we can expand.