Queue, Deque and Pair #39
Conversation
Demo
fixed name
demo
Reviewer's Guide by SourceryThis pull request introduces three new classes: UDeque, UQueue, and UPair, each with their respective methods for simplified operations. Comprehensive documentation and usage examples for each class are provided in new markdown files. 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 @TheHakerTech - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
Here's what I looked at during the review
- 🔴 General issues: 2 blocking issues, 8 other issues
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 13 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
|
Because of stack, queue, pair and deque are from c++ stl, i want to move all this features in |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @TheHakerTech - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 10 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 7 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| def __init__(self, *__list): | ||
| self.__list = list(__list) | ||
|
|
||
| def addend(self, value: VT) -> VT: |
There was a problem hiding this comment.
issue: Redundant check for self in addend method
The check if self: is redundant in the addend method. The method should always append the value to the list without checking if the deque is non-empty.
| else: | ||
| raise IndexError("index out of range") | ||
|
|
||
| def addbegin(self, value: VT) -> VT: |
There was a problem hiding this comment.
issue: Redundant check for self in addbegin method
The check if self: is redundant in the addbegin method. The method should always insert the value at the beginning of the list without checking if the deque is non-empty.
ufpy/udeque.py
Outdated
| def __getitem__(self, index: Any) -> VT: | ||
| if isinstance(index, int): | ||
| if index in (0, 1): | ||
| return self.__list[index] | ||
| else: | ||
| raise IndexError(f"{index} out of range (0, 1)") |
There was a problem hiding this comment.
issue (bug_risk): Incorrect index range check in getitem
The index range check in __getitem__ should allow for any valid index within the list, not just 0 and 1. This will cause an IndexError for valid indices beyond 1.
ufpy/udeque.py
Outdated
| if index in (0, 1): | ||
| return self.__list[index] | ||
| else: | ||
| raise IndexError(f"{index} out of range (0, 1)") |
There was a problem hiding this comment.
issue (bug_risk): Incorrect index range check in setitem
The index range check in __setitem__ should allow for any valid index within the list, not just 0 and 1. This will cause an IndexError for valid indices beyond 1.
ufpy/udeque.py
Outdated
| if index in (0, 1): | ||
| return self.__list[index] | ||
| else: | ||
| raise IndexError(f"{index} out of range (0, 1)") |
There was a problem hiding this comment.
issue (bug_risk): Incorrect index range check in delitem
The index range check in __delitem__ should allow for any valid index within the list, not just 0 and 1. This will cause an IndexError for valid indices beyond 1.
| ``` | ||
|
|
||
| > [!NOTE] | ||
| > After using `for` your deque will be empty: |
There was a problem hiding this comment.
question (documentation): Question on UDeque Iteration Section
Is it necessary to mention that the deque will be empty after iteration? This behavior might be unexpected and should be highlighted more prominently if it is indeed the case.
| ``` | ||
|
|
||
| > [!NOTE] | ||
| > After using `for` your queue will be empty: |
There was a problem hiding this comment.
question (documentation): Question on UQueue Iteration Section
Is it necessary to mention that the queue will be empty after iteration? This behavior might be unexpected and should be highlighted more prominently if it is indeed the case.
ufpy/udeque.py
Outdated
| value = self.popbegin() | ||
| return value |
There was a problem hiding this comment.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
| value = self.popbegin() | |
| return value | |
| return self.popbegin() |
bleudev
dismissed
sourcery-ai’s stale review
He requested changes, which must be fixed
closes #35
Summary by Sourcery
This pull request introduces three new utility classes:
UDeque,UQueue, andUPair, each designed to simplify common operations for double-ended queues, queues, and pairs respectively. Comprehensive documentation for each class has also been added.UDequeclass to simplify working with double-ended queues, including methods for adding, removing, and accessing elements at both ends.UQueueclass to simplify working with queues, including methods for adding, removing, and accessing the head element.UPairclass to simplify working with pairs, including methods for accessing and setting the first and second elements.UDequeinexamples/udeque.mdcovering creation, element access, addition, deletion, and iteration.UQueueinexamples/uqueue.mdcovering creation, element access, addition, deletion, and iteration.UPairinexamples/upair.mdcovering creation, element access, setting values, and length calculation.