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

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

Solution #1445

wants to merge 2 commits into from

Conversation

sumseer
Copy link

@sumseer sumseer commented Oct 2, 2024

No description provided.

app/main.py Outdated

def __truediv__(self, other: int | float) -> "Distance":
if isinstance(other, (int, float)):
return Distance(round(self.km / other, 2))
Copy link

Choose a reason for hiding this comment

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

in metods "mul" and "truediv" you dont need to check "other" for int/float types, because he not be of type Distance

Choose a reason for hiding this comment

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

👍 fix this

app/main.py Outdated
if isinstance(other, Distance):
return self.km < other.km
elif isinstance(other, (int, float)):
return self.km < other
Copy link

Choose a reason for hiding this comment

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

you can skip "elif" her and below and just use "return"

Choose a reason for hiding this comment

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

👍 fix this in all cases

Also add annotation for Disntace in other argument

app/main.py Outdated
Comment on lines 2 to 3
def __init__(self, km: float) -> None:
self.km = km

Choose a reason for hiding this comment

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

km can also be int

app/main.py Outdated
def __repr__(self) -> str:
return f"Distance(km={self.km})"

def __add__(self, other: int | float) -> "Distance":

Choose a reason for hiding this comment

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

other can be Distance type here

app/main.py Outdated
Comment on lines 12 to 15
if isinstance(other, Distance):
return Distance(self.km + other.km)
elif isinstance(other, (int, float)):
return Distance(self.km + other)

Choose a reason for hiding this comment

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

We don't need to check for type twice. If it's not Distance -> it will be float or int type

app/main.py Outdated
elif isinstance(other, (int, float)):
return Distance(self.km + other)

def __iadd__(self, other: int | float) -> "Distance":

Choose a reason for hiding this comment

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

Import annotations from future to use classes as type hints without quotes

app/main.py Outdated

def __truediv__(self, other: int | float) -> "Distance":
if isinstance(other, (int, float)):
return Distance(round(self.km / other, 2))

Choose a reason for hiding this comment

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

👍 fix this

app/main.py Outdated
if isinstance(other, Distance):
return self.km < other.km
elif isinstance(other, (int, float)):
return self.km < other

Choose a reason for hiding this comment

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

👍 fix this in all cases

Also add annotation for Disntace in other argument

Copy link

@AnyoneClown AnyoneClown left a comment

Choose a reason for hiding this comment

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

LGTM

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