solution #670
solution #670
Conversation
There was a problem hiding this comment.
Great job on implementing the task requirements! 🎉 While there are a few areas for improvement, such as using the IntegerRange descriptors correctly in the can_access method and providing more descriptive error messages, these are minor issues that don't significantly impact the functionality. Keep up the good work and consider these suggestions for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
|
|
||
| def __set__(self, instance: object, value: int) -> None: | ||
| if not isinstance(value, int): | ||
| raise TypeError |
There was a problem hiding this comment.
Consider providing a more descriptive error message when raising the TypeError to indicate that the value must be an integer.
| if self.min_amount <= value <= self.max_amount: | ||
| setattr(instance, self.protected_name, value) | ||
| else: | ||
| setattr(instance, self.protected_name, None) |
There was a problem hiding this comment.
Instead of setting the attribute to None when the value is out of range, it would be more appropriate to raise a ValueError with a descriptive message. This will make it clear that the value is invalid and why.
| self.limitation_class = limitation_class | ||
|
|
||
| def can_access(self, man: Visitor) -> bool: | ||
| access = self.limitation_class(man.age, man.weight, man.height) |
There was a problem hiding this comment.
The can_access method currently creates an instance of the limitation class with the visitor's attributes. However, the IntegerRange descriptors are not being used correctly. You should check if the visitor's attributes are within the allowed range using the descriptors, rather than relying on the instantiation of the limitation class.
No description provided.