-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⚡ update cex_exchange_plugin.py and dex_exchange_plugin.py #1434
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mraniki - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
if settings.bot_ignore not in msg or settings.bot_prefix not in msg: | ||
if await self.fmo.search(msg) and self.should_handle_timeframe(): | ||
order = await self.fmo.get_order(msg) | ||
if order and settings.trading_enabled: | ||
trade = await self.exchange.submit_order(order) | ||
if trade: | ||
await send_notification(trade) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs
)
if settings.bot_ignore not in msg or settings.bot_prefix not in msg: | |
if await self.fmo.search(msg) and self.should_handle_timeframe(): | |
order = await self.fmo.get_order(msg) | |
if order and settings.trading_enabled: | |
trade = await self.exchange.submit_order(order) | |
if trade: | |
await send_notification(trade) | |
if (settings.bot_ignore not in msg or settings.bot_prefix not in msg) and (await self.fmo.search(msg) and self.should_handle_timeframe()): | |
order = await self.fmo.get_order(msg) | |
if order and settings.trading_enabled: | |
trade = await self.exchange.submit_order(order) | |
if trade: | |
await send_notification(trade) | |
Explanation
Too much nesting can make code difficult to understand, and this is especiallytrue in Python, where there are no brackets to help out with the delineation of
different nesting levels.
Reading deeply nested code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two if
conditions can be combined using
and
is an easy win.
if settings.bot_ignore not in msg or settings.bot_prefix not in msg: | ||
if await self.fmo.search(msg) and self.should_handle_timeframe(): | ||
order = await self.fmo.get_order(msg) | ||
if order and settings.trading_enabled: | ||
trade = await self.exchange.submit_order(order) | ||
if trade: | ||
await send_notification(trade) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs
)
if settings.bot_ignore not in msg or settings.bot_prefix not in msg: | |
if await self.fmo.search(msg) and self.should_handle_timeframe(): | |
order = await self.fmo.get_order(msg) | |
if order and settings.trading_enabled: | |
trade = await self.exchange.submit_order(order) | |
if trade: | |
await send_notification(trade) | |
if (settings.bot_ignore not in msg or settings.bot_prefix not in msg) and (await self.fmo.search(msg) and self.should_handle_timeframe()): | |
order = await self.fmo.get_order(msg) | |
if order and settings.trading_enabled: | |
trade = await self.exchange.submit_order(order) | |
if trade: | |
await send_notification(trade) | |
Explanation
Too much nesting can make code difficult to understand, and this is especiallytrue in Python, where there are no brackets to help out with the delineation of
different nesting levels.
Reading deeply nested code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two if
conditions can be combined using
and
is an easy win.
No description provided.