Предупреждать, если get_all используется с методом, не кончающимся на *.list #225
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leshchenko1979 - I've reviewed your changes and they look great!
General suggestions:
- Consider raising an exception instead of logging a warning to enforce correct usage patterns more strictly.
- The implementation of the warning is a good step towards improving code maintainability and guiding developers. However, evaluating the impact of this change on existing codebases and ensuring it aligns with overall error handling strategies could be beneficial.
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
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| if not self.st_method.endswith(".list"): | ||
| logger.logger.warning( | ||
| "get_all() should be used only with methods that end with '.list'. " | ||
| "Use get_by_ID() or call() instead." | ||
| ) |
There was a problem hiding this comment.
suggestion (code_refinement): Consider raising an exception for unsupported methods.
Logging a warning is helpful, but in cases where the method is used incorrectly, raising an exception might prevent further execution with an unsupported method, enforcing correct usage patterns more strictly.
| @@ -105,6 +105,12 @@ | |||
| "https://github.com/leshchenko1979/fast_bitrix24/issues/199", | |||
| ) | |||
| def check_special_limitations(self): | |||
| if not self.st_method.endswith(".list"): | |||
There was a problem hiding this comment.
issue (complexity): Consider the addition of a warning for non '.list' method usage with get_all() as a beneficial enhancement.
The addition of a warning for methods not ending in ".list" is a thoughtful way to guide correct usage of get_all(). This change introduces minimal complexity, focusing on enhancing maintainability by alerting developers to potential misuse. It's a straightforward and effective update that doesn't significantly increase cognitive load or code complexity. Good job on keeping the implementation simple and focused on improving developer experience.
No description provided.