Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Solution' #609

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Andrey-Dementyev
Copy link

No description provided.

app/main.py Outdated
self.min_amount = min_amount
self.max_amount = max_amount

def __set_name__(self, owner: type, name: str) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def __set_name__(self, owner: type, name: str) -> None:
def __set_name__(self, owner: SlideLimitationValidator, name: str) -> None:

app/main.py Outdated
def __get__(self, instance: object, owner: type) -> int:
return getattr(instance, self.protected_name)

def __set__(self, instance: object, value: int) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specify instance type

app/main.py Outdated
def __set_name__(self, owner: type, name: str) -> None:
self.protected_name = f"_{name}"

def __get__(self, instance: object, owner: type) -> int:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

app/main.py Outdated


class Slide:
pass
def __init__(self, name: str, limitation_class: type) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specify limitation_class

Copy link

@Arsen-hrynevych Arsen-hrynevych left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly correct.

self.weight = weight
self.height = height


class IntegerRange:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IntegerRange descriptor is supposed to work with class attributes in SlideLimitationValidator (age, height, weight), but the current implementation only sets and gets instance attributes.

self.name = name
self.limitation_class = limitation_class

def can_access(self, visitor: Visitor) -> bool:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the can_access method of the Slide class, you instantiate the limitation_class, but you don't check whether the visitor's parameters fit within the range. To properly verify this, you should instantiate the class and then allow the descriptors (age, height, and weight) to validate the visitor's properties by assigning them one by one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants