-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Hw is done please check #911
base: master
Are you sure you want to change the base?
Conversation
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.
Great work on your code! 👍 You've followed the task requirements and best practices well by ensuring encapsulation in the Truck
class and using generics effectively in BulldozerProducer
. Keep up the good work, and remember to maintain clean code formatting and proper access modifiers in your classes. Keep coding and learning! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
public class Bulldozer extends Machine { | ||
private int weight; |
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.
Don't forget about access modifiers for your custom fields in Bulldozer
class. They should be private to prevent direct access from outside of the class.
public class Bulldozer extends Machine { | ||
private int weight; | ||
private int height; |
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.
Don't forget about access modifiers for your custom fields in Bulldozer
class. They should be private to prevent direct access from outside of the class.
@@ -5,9 +5,28 @@ | |||
* Do not remove no-args constructor | |||
*/ | |||
public class Excavator extends 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.
Don't begin class or method implementation with empty line.
private double fuelTankCap; | ||
private int boomLength; |
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.
Don't forget about access modifiers for your custom fields in Excavator
class. It's best practice to keep them private and provide getters and setters for accessing these fields.
@@ -5,9 +5,28 @@ | |||
* Do not remove no-field constructor | |||
*/ | |||
public class Truck extends 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.
Don't begin class or method implementation with empty line. Remove the empty lines between the package statement and the class declaration.
import java.util.List; | ||
|
||
public interface MachineProducer { | ||
public interface MachineProducer<T extends 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.
Don't begin class or method implementation with an empty line. The interface declaration should be followed by the method declaration without an empty line in between.
/** | ||
* Your implementation of MachineService. | ||
*/ | ||
public class MachineServiceImpl { | ||
public class MachineServiceImpl<T extends Machine> implements MachineService<T> { |
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.
Don't begin class or method implementation with empty line.
public List<T> getAll(Class<? extends T> type) { | ||
if (type == Bulldozer.class) { | ||
MachineProducer<Bulldozer> bulldozerProducer = new BulldozerProducer(); | ||
return (List<T>) bulldozerProducer.get(); | ||
} else if (type == Excavator.class) { | ||
MachineProducer<Excavator> excavatorProducer = new ExcavatorProducer(); | ||
return (List<T>) excavatorProducer.get(); | ||
} else if (type == Truck.class) { | ||
MachineProducer<Truck> truckProducer = new TruckProducer(); | ||
return (List<T>) truckProducer.get(); | ||
} | ||
return List.of(); |
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.
Consider creating a local variable in your implementation of getAll(Class type)
method to store the result of get()
method and then return it, instead of casting and returning in one line.
if (type == Bulldozer.class) { | ||
MachineProducer<Bulldozer> bulldozerProducer = new BulldozerProducer(); | ||
return (List<T>) bulldozerProducer.get(); | ||
} else if (type == Excavator.class) { | ||
MachineProducer<Excavator> excavatorProducer = new ExcavatorProducer(); | ||
return (List<T>) excavatorProducer.get(); | ||
} else if (type == Truck.class) { | ||
MachineProducer<Truck> truckProducer = new TruckProducer(); | ||
return (List<T>) truckProducer.get(); |
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.
The getAll
method should not return null or a specific type List when no producer has been found. Instead, it should return an empty List. The current implementation already follows this, but make sure to avoid returning null or a specific type list in future changes.
public List<T> getAll(Class<? extends T> type) { | ||
if (type == Bulldozer.class) { | ||
MachineProducer<Bulldozer> bulldozerProducer = new BulldozerProducer(); | ||
return (List<T>) bulldozerProducer.get(); | ||
} else if (type == Excavator.class) { | ||
MachineProducer<Excavator> excavatorProducer = new ExcavatorProducer(); | ||
return (List<T>) excavatorProducer.get(); | ||
} else if (type == Truck.class) { | ||
MachineProducer<Truck> truckProducer = new TruckProducer(); | ||
return (List<T>) truckProducer.get(); | ||
} | ||
return List.of(); |
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.
Keep if-else constructions simple. There is no need for else after return in if clause. You can remove the else keyword to make the code cleaner.
No description provided.