Skip to content

Implement Distance class with arithmetic and comparison methods#2794

Open
naderidkeidek-star wants to merge 1 commit into
mate-academy:masterfrom
naderidkeidek-star:master
Open

Implement Distance class with arithmetic and comparison methods#2794
naderidkeidek-star wants to merge 1 commit into
mate-academy:masterfrom
naderidkeidek-star:master

Conversation

@naderidkeidek-star

Copy link
Copy Markdown

1st Try

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your implementation is functionally correct and all operations work as expected. However, there are explicit checklist violations that must be addressed: Missing type annotations on all magic method parameters (like other) and return types, and an unnecessary else statement in __iadd__ (line 19). The checklist explicitly requires type annotations using other: int | float | Distance syntax for arithmetic methods and other: int | float for __mul__ and __truediv__. These are not cosmetic issues—they're documented requirements that the linter is flagging. Add from __future__ import annotations at the top of the file, then annotate all parameters and return types. For the __iadd__ method, remove the else block and return directly after the if body.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread app/main.py
def __repr__(self):
return f"Distance(km={self.km})"

def __add__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for method arguments. The checklist requires proper type annotations like other: int | float | Distance for all magic methods that accept other parameter.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

class Distance:
def init(self, km: int | float) -> None:
self.km = km

def __str__(self) -> str:
    return f"Distance: {self.km} kilometers."

def __repr__(self) -> str:
    return f"Distance(km={self.km})"

def __add__(self, other: int | float | "Distance") -> "Distance":
    if isinstance(other, Distance):
        return Distance(self.km + other.km)
    return Distance(self.km + other)

def __iadd__(self, other: int | float | "Distance") -> "Distance":
    if isinstance(other, Distance):
        self.km += other.km
    else:
        self.km += other
    return self

def __mul__(self, other: int | float) -> "Distance":
    return Distance(self.km * other)

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

def __lt__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km < other.km
    return self.km < other

def __gt__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km > other.km
    return self.km > other

def __eq__(self, other: object) -> bool:
    if isinstance(other, Distance):
        return self.km == other.km
    if isinstance(other, (int, float)):
        return self.km == other
    return False

def __le__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km <= other.km
    return self.km <= other

def __ge__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km >= other.km
    return self.km >= other

Comment thread app/main.py
Comment on lines +11 to +14
def __add__(self, other):
if isinstance(other, Distance):
return Distance(self.km + other.km)
return Distance(self.km + other)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for method arguments. Add -> Distance return type annotation.

Comment thread app/main.py
return Distance(self.km + other.km)
return Distance(self.km + other)

def __iadd__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for other parameter.

Comment thread app/main.py
Comment on lines +18 to +20
self.km += other.km
else:
self.km += other

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checklist item #1 violation: Avoid unnecessary else statements. The else on line 19 is redundant here.

Comment thread app/main.py
self.km += other
return self

def __mul__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for other parameter. The checklist specifies these methods should not accept Distance as argument, so it should be other: int | float.

Comment thread app/main.py
def __truediv__(self, other):
return Distance(round(self.km / other, 2))

def __lt__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for other parameter.

Comment thread app/main.py
return self.km < other.km
return self.km < other

def __gt__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for other parameter.

Comment thread app/main.py
return self.km > other.km
return self.km > other

def __eq__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for other parameter.

Comment thread app/main.py
return self.km == other.km
return self.km == other

def __le__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for other parameter.

Comment thread app/main.py
return self.km <= other.km
return self.km <= other

def __ge__(self, other):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing type annotations for other parameter.

@naderidkeidek-star naderidkeidek-star left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

2nd Try

Comment thread app/main.py
def __repr__(self):
return f"Distance(km={self.km})"

def __add__(self, other):

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

class Distance:
def init(self, km: int | float) -> None:
self.km = km

def __str__(self) -> str:
    return f"Distance: {self.km} kilometers."

def __repr__(self) -> str:
    return f"Distance(km={self.km})"

def __add__(self, other: int | float | "Distance") -> "Distance":
    if isinstance(other, Distance):
        return Distance(self.km + other.km)
    return Distance(self.km + other)

def __iadd__(self, other: int | float | "Distance") -> "Distance":
    if isinstance(other, Distance):
        self.km += other.km
    else:
        self.km += other
    return self

def __mul__(self, other: int | float) -> "Distance":
    return Distance(self.km * other)

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

def __lt__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km < other.km
    return self.km < other

def __gt__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km > other.km
    return self.km > other

def __eq__(self, other: object) -> bool:
    if isinstance(other, Distance):
        return self.km == other.km
    if isinstance(other, (int, float)):
        return self.km == other
    return False

def __le__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km <= other.km
    return self.km <= other

def __ge__(self, other: int | float | "Distance") -> bool:
    if isinstance(other, Distance):
        return self.km >= other.km
    return self.km >= other

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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.

2 participants