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 #270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Solution #270

wants to merge 1 commit into from

Conversation

dminyaylo
Copy link

No description provided.

self.max_amount = max_amount

def __get__(self, instance: object, owner: type) -> None:
return instance.__dict__[self.name]
Copy link

Choose a reason for hiding this comment

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

use getattr

if not self.min_amount <= value <= self.max_amount:
raise ValueError(f"{self.name} must be between "
f"{self.min_amount} and {self.max_amount}")
instance.__dict__[self.name] = value
Copy link

Choose a reason for hiding this comment

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

use setattr

Copy link
Author

Choose a reason for hiding this comment

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

RecursionError: maximum recursion depth exceeded
Куча таких ошибок с setattr и getattr

Choose a reason for hiding this comment

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

In this case you need have two different names: public and protected

@dminyaylo
Copy link
Author

RecursionError: maximum recursion depth exceeded
Куча таких ошибок с setattr и getattr

@dminyaylo dminyaylo requested a review from vsmutok August 20, 2023 18:29
self.max_amount = max_amount

def __get__(self, instance: object, owner: type) -> None:
return instance.__dict__[self.name]

Choose a reason for hiding this comment

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

Use getattr

if not self.min_amount <= value <= self.max_amount:
raise ValueError(f"{self.name} must be between "
f"{self.min_amount} and {self.max_amount}")
instance.__dict__[self.name] = value

Choose a reason for hiding this comment

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

In this case you need have two different names: public and protected

instance.__dict__[self.name] = value

def __set_name__(self, owner: type, name: str) -> None:
self.name = name

Choose a reason for hiding this comment

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

Suggested change
self.name = name
self.name = f"_{name}"

Choose a reason for hiding this comment

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

Or just fix like this

Comment on lines +29 to +31
age: IntegerRange = IntegerRange(0, 150)
weight: IntegerRange = IntegerRange(0, 500)
height: IntegerRange = IntegerRange(0, 300)

Choose a reason for hiding this comment

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

This is redundant. Remove this lines

Comment on lines +40 to +42
@abstractmethod
def validate(self, visitor: Visitor) -> bool:
pass

Choose a reason for hiding this comment

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

This method is redundant

Choose a reason for hiding this comment

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

Remove in all classes



class ChildrenSlideLimitationValidator(SlideLimitationValidator):
pass
def __init__(self) -> None:
super().__init__(age=(4, 14), weight=(20, 50), height=(80, 120))

Choose a reason for hiding this comment

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

age, weight, height must be class attribute as IntegerRange



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

Choose a reason for hiding this comment

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

type of what? Provide inner type

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