refactor: replace class-based handlers with function-based handlers #262
Conversation
Reviewer's Guide by SourceryThis pull request refactors the entire codebase to replace class-based handlers with function-based handlers. The changes aim to simplify the architecture, improve maintainability, and streamline the code structure. The refactoring affects various modules, including handlers for commands, callbacks, and error handling. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @HitaloM - 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: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
|
@sourcery-ai review |
HitaloM
changed the title
There was a problem hiding this comment.
Hey @HitaloM - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @HitaloM - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @HitaloM - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| return None | ||
|
|
||
|
|
||
| async def send_media( |
There was a problem hiding this comment.
suggestion: Consider breaking down the send_media function into smaller helper functions
The send_media function contains a lot of conditional logic. Consider breaking it down into smaller helper functions to improve readability and maintainability. This could also make the code easier to test.
async def send_media(client: Client, message: Message):
return await _process_and_send_media(client, message)
async def _process_and_send_media(client: Client, message: Message):
src/korone/modules/core.py
Outdated
|
|
||
| async def register_handler(client: Client, func: FunctionType) -> bool: |
There was a problem hiding this comment.
suggestion: Consider adding type hints for the 'handlers' attribute
To improve type checking and code clarity, consider adding type hints for the 'handlers' attribute of the func parameter. This could be done using a Protocol or a custom type.
from typing import Protocol, Callable
class HandlerProtocol(Protocol):
handlers: list[Callable]
async def register_handler(client: Client, func: HandlerProtocol) -> bool:
successful = False
| @@ -20,7 +20,7 @@ class TwitterError(Exception): | |||
| async def fetch_tweet(url: str) -> Tweet | None: | |||
| fx_url = re.sub(r"(www\.|)(twitter\.com|x\.com)", "api.fxtwitter.com", url) | |||
| try: | |||
| async with httpx.AsyncClient(http2=True) as client: | |||
| async with httpx.AsyncClient(http2=True, timeout=20) as client: | |||
There was a problem hiding this comment.
suggestion (performance): Consider if 20 seconds is an appropriate timeout for this API call
| async with httpx.AsyncClient(http2=True, timeout=20) as client: | |
| async with httpx.AsyncClient(http2=True, timeout=10) as client: |
| @@ -0,0 +1,226 @@ | |||
| # SPDX-License-Identifier: BSD-3-Clause | |||
There was a problem hiding this comment.
suggestion: Consider breaking down the functionality in kang.py into smaller, more manageable functions
# SPDX-License-Identifier: BSD-3-Clause
# Copyright (c) 2024 Hitalo M. <https://github.com/HitaloM>
from typing import List
def process_sticker(sticker_data: bytes) -> bytes:
# Process sticker logic here
pass
def create_sticker_set(name: str, stickers: List[bytes]) -> bool:
# Create sticker set logic here
pass
| await message.reply(_("Reply to a message to get the compatibility!")) | ||
| return | ||
| @router.message(Command("lfmcompat")) | ||
| async def lfmcompat_command(client: Client, message: Message) -> None: # noqa: C901, PLR0912 |
There was a problem hiding this comment.
issue (code-quality): Low code quality found in lfmcompat_command - 23% (low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
HitaloM
changed the title
Summary by Sourcery
Refactor the codebase to replace class-based handlers with function-based handlers, enhancing the handling of commands and events. Introduce new features for media handling and LastFM integration, and improve error handling and configuration management. Update the pre-commit configuration for better code quality checks.
New Features:
Enhancements:
Build: