-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Зробив завдання jv-oop #2481
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
base: master
Are you sure you want to change the base?
Зробив завдання jv-oop #2481
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
Excellent work on this task! Your solution correctly implements all the core requirements, demonstrating a solid understanding of inheritance and polymorphism. I am approving your submission. I have a couple of minor suggestions for you to consider for future projects:
- In
src/main/java/core/basesyntax/Machine.java, theMachineclass has a default (package-private) access modifier. It's a common convention to make base classespublicto allow them to be extended by classes in other packages. This is a good practice for creating reusable code. - In
src/main/java/core/basesyntax/MainApp.java, there is an extraSystem.out.println();that prints an unnecessary empty line.
These are just minor points for improvement and don't affect the functionality. Overall, this is a great implementation. Keep up the good work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| public static void main(String[] args) { | ||
| Machine[] machines = {new Truck(), new Bulldozer(), new Excavator()}; | ||
| System.out.println(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line prints an extra empty line to the console, which is not required by the task description. It's best to only include code that directly contributes to solving the problem.
| @@ -0,0 +1,8 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| abstract class Machine { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class has a default (package-private) access modifier. For a base class that is meant to be extended, it's generally better practice to declare it as public to ensure it can be accessed and subclassed from any other package. This relates to checklist item #4 about using appropriate access modifiers.
sarakhmen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job!
|
|
||
| public static void main(String[] args) { | ||
| Machine[] machines = {new Truck(), new Bulldozer(), new Excavator()}; | ||
| System.out.println(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's remove this System.out.println();
No description provided.