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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 77 additions & 1 deletion app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,80 @@ class Cargo:
def __init__(self, weight: int) -> None:
self.weight = weight

# write your code here

class BaseRobot:
def __init__(
self, name: str, weight: int, coords: list[int] = None

Choose a reason for hiding this comment

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

It's better to avoid mutable default arguments like list in function definitions, as they can lead to unexpected results. A better approach would be to set the default value to None and then assign the default value within the function if it's not provided.

) -> None:
if coords is None:
coords = [0, 0]
Comment on lines +8 to +11

Choose a reason for hiding this comment

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

The default mutable arguments in function definitions can lead to unexpected behavior. In Python, default mutable objects are shared between subsequent function calls which can lead to unexpected behaviour. It would be better to use a non-mutable default argument, such as None, and then assign the default value within the function if no value was provided.

Comment on lines +8 to +11

Choose a reason for hiding this comment

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

Code duplication: The initialization of coords is duplicated in the BaseRobot, FlyingRobot, and DeliveryDrone classes. You can move this logic to the BaseRobot class and remove it from the other classes.

self.name = name
self.weight = weight
self.coords = coords

def go_forward(self, step: int = 1) -> list[int]:
self.coords[1] += step
return self.coords

def go_back(self, step: int = 1) -> list[int]:
self.coords[1] -= step
return self.coords

def go_right(self, step: int = 1) -> list[int]:
self.coords[0] += step
return self.coords

def go_left(self, step: int = 1) -> list[int]:
self.coords[0] -= step
return self.coords
Comment on lines +16 to +30

Choose a reason for hiding this comment

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

Code duplication: The methods go_forward, go_back, go_right, and go_left are identical except for the index and operation. You could generalize this into a single method that takes the index and operation as parameters.


def get_info(self) -> str:
return f"Robot: {self.name}, Weight: {self.weight}"


class FlyingRobot(BaseRobot):
def __init__(
self, name: str, weight: int, coords: list[int] = None

Choose a reason for hiding this comment

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

Same issue with mutable default arguments as in the BaseRobot class. Please, avoid using mutable types as default values.

) -> None:
if coords is None:
coords = [0, 0, 0]
super().__init__(name, weight, coords)

def go_up(self, step: int = 1) -> list[int]:
self.coords[2] += step
return self.coords

def go_down(self, step: int = 1) -> list[int]:
self.coords[2] -= step
return self.coords


class DeliveryDrone(FlyingRobot):
def __init__(
self, name: str, weight: int, max_load_weight: int,
coords: list[int] = None, current_load: Cargo = None
) -> None:
if coords is None:
coords = [0, 0, 0]
super().__init__(name, weight, coords)
self.max_load_weight = max_load_weight
self.current_load = current_load

def hook_load(self, cargo: Cargo) -> str:
if self.current_load is not None:
return "Cannot hook load. There is already cargo hooked."
elif cargo.weight > self.max_load_weight:
return (
f"Cannot hook load. The cargo is too heavy: "
f"{cargo.weight} > {self.max_load_weight}."
)
self.current_load = cargo
return f"Cargo weighing {cargo.weight} hooked successfully."
Comment on lines +64 to +73

Choose a reason for hiding this comment

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

The hook_load method currently returns a string to indicate the status of the operation. This is not a common practice in Python. It would be better to raise an exception in case of an error. This way, the calling code can decide what to do in case of an error (e.g., show a message to the user, try again, etc.).

Comment on lines +64 to +73

Choose a reason for hiding this comment

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

Code quality: The hook_load method returns a string message. It would be better if it raises exceptions in case of errors. This way, the caller of the method can decide how to handle these exceptions, which is more flexible and idiomatic.


def unhook_load(self) -> str:
if self.current_load is None:
return "No cargo to unhook."

unhooked_cargo = self.current_load
self.current_load = None
return f"Cargo weighing {unhooked_cargo.weight} unhooked successfully."
Comment on lines +75 to +81

Choose a reason for hiding this comment

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

Similar to the hook_load method, the unhook_load method should raise an exception instead of returning a string in case of an error.

Comment on lines +75 to +81

Choose a reason for hiding this comment

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

Code quality: The unhook_load method also returns a string message. Similar to hook_load, it would be better to raise an exception in case of an error.

Loading