-
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
implemented pecs #919
base: master
Are you sure you want to change the base?
implemented pecs #919
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package core.mate.academy.model; | ||
|
||
import core.mate.academy.service.MachineProducer; | ||
import java.util.List; | ||
|
||
public class BulldozerProducer implements MachineProducer<Bulldozer> { | ||
@Override | ||
public List<Bulldozer> get() { | ||
return List.of(new Bulldozer()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package core.mate.academy.model; | ||
|
||
import core.mate.academy.service.MachineProducer; | ||
import java.util.List; | ||
|
||
public class ExcavatorProducer implements MachineProducer<Excavator> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with an empty line. Remove the empty line before the class declaration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with an empty line. Remove the empty line at the beginning of the class definition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with an empty line. |
||
@Override | ||
public List<Excavator> get() { | ||
return List.of(new Excavator()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
|
||
package core.mate.academy.model; | ||
|
||
/** | ||
* Add some custom fields that could be only in Truck | ||
* Do not remove no-field constructor | ||
*/ | ||
|
||
public class Truck extends Machine { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't forget about access modifiers for your custom fields in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The class There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with an empty line. It's good practice to start the class body immediately after the class declaration line to avoid unnecessary whitespace. |
||
public Truck() { | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package core.mate.academy.model; | ||
|
||
import core.mate.academy.service.MachineProducer; | ||
import java.util.List; | ||
|
||
public class TruckProducer implements MachineProducer<Truck> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with an empty line. The empty line after the class declaration is unnecessary and should be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with an empty line. Remove the empty line before the class declaration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with an empty line. Remove the empty line at the start of the class definition. |
||
@Override | ||
public List<Truck> get() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin method implementation with an empty line. Remove the empty line before the method declaration. |
||
return List.of(new Truck()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider creating a local variable for the list of machines before returning it. This can make debugging easier if you need to inspect the list before it is returned or if additional processing is required in the future. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,10 @@ | |
|
||
import java.util.List; | ||
|
||
public interface MachineProducer { | ||
public interface MachineProducer<T> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parameterize |
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin interface or method implementation with an empty line. It is unnecessary and does not follow Java code style conventions. |
||
* In your implementations - create 2-3 models, add them to the list and return | ||
* @return - the list of models | ||
*/ | ||
List<Object> get(); | ||
List<T> get(); | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,12 +1,13 @@ | ||||||
package core.mate.academy.service; | ||||||
|
||||||
import core.mate.academy.model.Machine; | ||||||
import java.util.List; | ||||||
|
||||||
/** | ||||||
* Parameterize this service and add its implementation in a separate class. | ||||||
*/ | ||||||
public interface MachineService { | ||||||
|
||||||
public interface MachineService<T> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin interface or method implementation with empty lines. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with empty lines. Please remove the excess empty lines to improve code readability. |
||||||
/** | ||||||
* Return the list of machines. | ||||||
* In the implementation of this method please use your MachineProducer implementations | ||||||
|
@@ -16,7 +17,7 @@ public interface MachineService { | |||||
* @param type - any class of Machine sub class. For example: Truck.class or Bulldozer.class | ||||||
* @return the list of machines | ||||||
*/ | ||||||
List<Object> getAll(Class type); | ||||||
List<T> getAll(Class<? extends T> type); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider parameterizing the |
||||||
|
||||||
/** | ||||||
* Fill the machines list with passed value | ||||||
|
@@ -26,13 +27,13 @@ public interface MachineService { | |||||
* @param machines - list of machines to be filled with value | ||||||
* @param value - any object of machine sub class | ||||||
*/ | ||||||
void fill(List<Object> machines, Object value); | ||||||
void fill(List<Object> machines, Machine value); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'fill' method should use the generic type 'T' for the 'machines' list instead of 'Object'. This ensures type safety. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
/** | ||||||
* Call the method doWork() from each machine. | ||||||
* This method should be able to accept a list of bulldozers as well as list of trucks. | ||||||
* | ||||||
* @param machines - the list of machines | ||||||
*/ | ||||||
void startWorking(List<Object> machines); | ||||||
void startWorking(List<? extends Machine> machines); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'startWorking' method is correctly using a bounded wildcard, but consider if it could be better to use the generic type 'T' as the parameter to ensure that the method works with the specific machine type that the service is intended for. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,7 +1,44 @@ | ||||||||||||||
package core.mate.academy.service; | ||||||||||||||
|
||||||||||||||
import core.mate.academy.model.Bulldozer; | ||||||||||||||
import core.mate.academy.model.BulldozerProducer; | ||||||||||||||
import core.mate.academy.model.Excavator; | ||||||||||||||
import core.mate.academy.model.ExcavatorProducer; | ||||||||||||||
import core.mate.academy.model.Machine; | ||||||||||||||
import core.mate.academy.model.Truck; | ||||||||||||||
import core.mate.academy.model.TruckProducer; | ||||||||||||||
import java.util.ArrayList; | ||||||||||||||
import java.util.List; | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* 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 commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with empty lines (lines 13-16). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't begin class or method implementation with empty lines (lines 13-15). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The class 'MachineServiceImpl' should not start with an empty line according to the common mistakes checklist. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
@SuppressWarnings("unchecked") | ||||||||||||||
@Override | ||||||||||||||
public List<T> getAll(Class<? extends T> type) { | ||||||||||||||
List<T> machines = new ArrayList<>(); | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
|
||||||||||||||
if (Truck.class.isAssignableFrom(type)) { | ||||||||||||||
machines.addAll((List<T>) new TruckProducer().get()); | ||||||||||||||
} else if (Bulldozer.class.isAssignableFrom(type)) { | ||||||||||||||
machines.addAll((List<T>) new BulldozerProducer().get()); | ||||||||||||||
} else if (Excavator.class.isAssignableFrom(type)) { | ||||||||||||||
machines.addAll((List<T>) new ExcavatorProducer().get()); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return machines; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'getAll' method should return an empty list instead of null when no producer has been found for the given type. Make sure to handle this case correctly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider creating a local variable in your implementation of |
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
@Override | ||||||||||||||
public void fill(List<Object> machines, Machine value) { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'fill' method should use List as a parameter instead of List to ensure type safety and avoid potential class cast issues at runtime. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'fill()' method should use a List of type T, not Object, to ensure type safety. Please correct the method signature to 'List machines' and adjust the implementation accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
machines.replaceAll(ignored -> value); | ||||||||||||||
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The size of a list passed to |
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
@Override | ||||||||||||||
public void startWorking(List<? extends Machine> machines) { | ||||||||||||||
for (Machine machine : machines) { | ||||||||||||||
machine.doWork(); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} |
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 line at the start of the class definition.