Solution #1238
Solution #1238
Conversation
There was a problem hiding this comment.
Great job so far! 😄 However, there are a few improvements needed before your code is ready. Please take a look at the comments about using the extend() method instead of += for adding multiple values to the skills list. Also, reconsider the need to override the create_web_application method in the FullStackDeveloper class if you're not adding any additional logic. Keep up the good work! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
app/main.py
Outdated
| class FrontendDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills += ["JavaScript", "HTML", "CSS"] |
There was a problem hiding this comment.
Checklist point 1: Instead of using += operator to add multiple values to the skills list, you should use the extend() method. This would make your code more aligned with the Python best practices. Here is how you can do it: self.skills.extend(["JavaScript", "HTML", "CSS"])
app/main.py
Outdated
| class BackendDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills += ["Python", "SQL", "Django"] |
There was a problem hiding this comment.
Checklist point 1: Instead of using += operator to add multiple values to the skills list, you should use the extend() method. This would make your code more aligned with the Python best practices. Here is how you can do it: self.skills.extend(["Python", "SQL", "Django"])
app/main.py
Outdated
| class AndroidDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills += ["Java" , "Android studio"] |
There was a problem hiding this comment.
Checklist point 1: Instead of using += operator to add multiple values to the skills list, you should use the extend() method. This would make your code more aligned with the Python best practices. Here is how you can do it: self.skills.extend(["Java" , "Android studio"])
app/main.py
Outdated
| def create_web_application(self) -> None: | ||
| print(f"{self.name} started creating a web application...") | ||
| self.create_powerful_api() | ||
| self.create_awesome_web_page() |
There was a problem hiding this comment.
Checklist point 3: You are overriding the create_web_application method in the FullStackDeveloper class without adding any additional logic. This is considered a bad practice. If you do not intend to add any additional logic, you should not override the method.
There was a problem hiding this comment.
Great work so far! 😊 However, there are few areas that need a bit of tweaking. Please, take into account the provided feedback about overriding methods and list manipulation. Keep going, you're almost there! 💪
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| def learn_skill(self, skill: str) -> None: | ||
| self.skills.append(skill) |
There was a problem hiding this comment.
According to the checklist, you should use the extend() method or += operator to add multiple values to a single list. You can replace append() with extend() or += and provide a list with a single element. This will make your code more consistent and easier to maintain.
| class FrontendDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills.extend(["JavaScript", "HTML", "CSS"]) |
There was a problem hiding this comment.
Good job using the extend() method to add multiple skills to the list. This is exactly what was required in the checklist.
| class BackendDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills.extend(["Python", "SQL", "Django"]) |
There was a problem hiding this comment.
Good job using the extend() method to add multiple skills to the list. This is exactly what was required in the checklist.
| class AndroidDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills.extend(["Java" , "Android studio"]) |
There was a problem hiding this comment.
Good job using the extend() method to add multiple skills to the list. This is exactly what was required in the checklist.
app/main.py
Outdated
| def create_web_application(self) -> None: | ||
| print(f"{self.name} started creating a web application...") | ||
| self.create_powerful_api() | ||
| self.create_awesome_web_page() |
There was a problem hiding this comment.
You are overriding the methods create_powerful_api() and create_awesome_web_page() from the parent classes BackendDeveloper and FrontendDeveloper respectively in the FullStackDeveloper class but you are not adding any additional logic inside. This is considered a bad practice according to the checklist. If you want to reuse the logic from the parent classes, you don't need to override the methods, you can directly call them.
There was a problem hiding this comment.
Great job on your code! There are a few areas that need attention, specifically the use of the extend() method and the overriding of the create_web_application() method. These issues should be addressed to make your code more efficient and maintainable. Keep up the good work and continue to learn from your mistakes! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| class FrontendDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills.extend(["JavaScript", "HTML", "CSS"]) |
There was a problem hiding this comment.
You use extend() method to add multiple values to a single list, which is a good practice according to the checklist point 1. However, the extend() method should be used in the learn_skill() method of the SoftwareEngineer class, not in the __init__() method of each subclass. It would be more efficient and maintainable if you add the skills for each specific type of engineer in the learn_skill() method of each subclass, instead of doing it in the __init__() method.
| class BackendDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills.extend(["Python", "SQL", "Django"]) |
There was a problem hiding this comment.
The same issue as in the FrontendDeveloper class. It's better to move the extend() method to the learn_skill() method.
| class AndroidDeveloper(SoftwareEngineer): | ||
| def __init__(self, name: str) -> None: | ||
| super().__init__(name) | ||
| self.skills.extend(["Java" , "Android studio"]) |
There was a problem hiding this comment.
The same issue as in the previous classes. It's better to move the extend() method to the learn_skill() method.
| def create_web_application(self) -> None: | ||
| print(f"{self.name} started creating a web application...") | ||
| super().create_powerful_api() | ||
| super().create_awesome_web_page() |
There was a problem hiding this comment.
It seems like you are overriding the create_web_application() method in the FullStackDeveloper class without adding any additional logic. This is not a good practice according to the checklist point 3. If you need to use the methods from the parent classes, you can just call them without overriding.
No description provided.