add empty Dictionary #1095
add empty Dictionary #1095
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
You've set up the Dictionary class, which is a great start. However, the implementation is currently missing, so I'm requesting changes to build out its functionality. The class is empty and lacks the core methods required by the task.
To move forward, please implement the following essential methods:
__init____setitem____getitem____len__
Please refer to the task description for details on how to build the underlying hash table and handle key-value pairs. I'm looking forward to reviewing your completed implementation!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
app/main.py
Outdated
| class Dictionary: | ||
| pass | ||
| pass |
There was a problem hiding this comment.
It looks like you've set up the Dictionary class, which is a great start. However, the implementation is missing. According to the task description, you need to implement the core methods like __init__, __setitem__, __getitem__, and __len__ to make it functional.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
You've made a great start on the Dictionary class by setting up __init__ and __len__ correctly! I am requesting changes because the core functionality for adding and retrieving items is missing or incomplete, which prevents the class from working as expected.
Here are the critical issues to fix:
- Incorrect Method Name: The method
__set_item__needs to be renamed to__setitem__. This specific name is required for Python'smy_dict[key] = valuesyntax to work. - Incomplete
__setitem__Logic: After calculating theindex, you need to add the logic to actually store the key-value pair in thehash_table. - Missing
__getitem__Method: The__getitem__method, which is essential for retrieving values (e.g.,value = my_dict[key]), has not been implemented.
Once these are addressed, your dictionary will be much closer to being fully functional. Keep up the good work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
app/main.py
Outdated
| def __len__(self) -> int: | ||
| return self.length | ||
|
|
||
| def __set_item__(self, key, value) -> None: |
There was a problem hiding this comment.
The name of this method is incorrect. For Python to correctly handle dictionary-style item assignment (e.g., my_dict[key] = value), the magic method must be named __setitem__ (with two underscores at the beginning and end, and no underscore in the middle).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution as an educational override to help you progress. You've done a great job implementing the __setitem__ logic and the overall class structure!
The next critical step is to correctly handle hash collisions. Currently, your __getitem__ and __delitem__ methods only check the initial hash index and don't search for the key if that slot is occupied by a different key. You'll need to implement a search loop, similar to your __setitem__ probing, to find the correct key. Additionally, in __delitem__, using del on the list item alters the list's structure and breaks the hashing logic. Instead of deleting the item, you should set its value to None to mark the slot as empty.
Please also review the linter feedback to fix the minor style issues. Keep up the fantastic work, you're on the right track!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
app/main.py
Outdated
| if self.__hash_table[index].key != key: | ||
| raise KeyError(f"Key {key} not found!") |
There was a problem hiding this comment.
This check doesn't account for hash collisions. If self.__hash_table[index] contains a different key, this code will incorrectly raise a KeyError instead of searching for the correct key in subsequent slots. You'll need a loop to probe for the key, which should continue until the key is found or an empty (None) slot is encountered.
app/main.py
Outdated
| if self.__hash_table[index].key != key: | ||
| raise KeyError(f"Key {key} not found!") |
There was a problem hiding this comment.
Similar to __getitem__, this check doesn't handle hash collisions. It needs to probe through the hash table to locate the correct key before it can be deleted, otherwise it might fail to find keys that were shifted due to a collision.
app/main.py
Outdated
| if self.__hash_table[index].key != key: | ||
| raise KeyError(f"Key {key} not found!") | ||
|
|
||
| del self.__hash_table[index] |
There was a problem hiding this comment.
Using del on a list item removes it and shifts all subsequent items, which invalidates the hash table's indices. To remove an item, you should set its slot back to None. However, simply setting it to None can create a 'hole' that breaks the linear probing chain for later lookups, which is an additional challenge to consider.
app/main.py
Outdated
| custom_dict = Dictionary() | ||
| custom_dict[8] = "8" | ||
| custom_dict[16] = "16" | ||
| custom_dict[32] = "32" | ||
| custom_dict["one"] = 1 | ||
| custom_dict["one"] = 11 | ||
| custom_dict["f"] = 60 | ||
| custom_dict["g"] = 70 | ||
| # value = custom_dict["a"] | ||
| # value2 = custom_dict["b"] |
There was a problem hiding this comment.
This violates checklist item #Clean Code: 'Don't forget to delete them when you are ready to commit and push your code.'
This block of test code should be removed from the final submission.
2 participants
No description provided.