implemented pecs #919

Nazar1106 · 2024-10-08T10:49:37Z

No description provided.

mateacademy-ai-mentor

Hey there! It looks like there are a few critical issues that need to be addressed in your code. The use of generics needs to be tightened up to ensure type safety, and the logic in methods like 'getAll' and 'startWorking' needs to be corrected to match the task's requirements. Also, remember to avoid using raw types and unnecessary class fields. Keep up the good work, and with a bit of tweaking, your code will be in great shape! 👍💻

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-10-08T10:50:06Z

src/main/java/core/mate/academy/model/Bulldozer.java

@@ -6,10 +6,13 @@
 */
 public class Bulldozer extends Machine {


Don't begin class or method implementation with empty lines (lines 3-6). The class declaration should follow directly after the package statement.

mateacademy-ai-mentor · 2024-10-08T10:50:06Z

src/main/java/core/mate/academy/model/Bulldozer.java

@@ -6,10 +6,13 @@
 */
 public class Bulldozer extends Machine {
 public Bulldozer() {
+ setName(getName());


The no-args constructor is setting the name of the Bulldozer to its current name which is not initialized yet. This logic doesn't seem to serve any purpose and could potentially be removed unless there's a specific reason for this implementation.

mateacademy-ai-mentor · 2024-10-08T10:50:06Z

src/main/java/core/mate/academy/model/Bulldozer.java

@@ -6,10 +6,13 @@
 */
 public class Bulldozer extends Machine {
 public Bulldozer() {


Don't remove the no-args constructor, but also make sure it doesn't contain unnecessary logic as currently seen in line 9.

mateacademy-ai-mentor · 2024-10-08T10:50:07Z

src/main/java/core/mate/academy/model/BulldozerProducer.java

+
+public class BulldozerProducer implements MachineProducer<Bulldozer> {
+ @Override
+ public List<? extends Machine> get() {


The return type of the get method should be parametrized with Bulldozer, not the raw Machine type to avoid using raw types and to ensure type safety.

mateacademy-ai-mentor · 2024-10-08T10:50:07Z

src/main/java/core/mate/academy/model/BulldozerProducer.java

+public class BulldozerProducer implements MachineProducer<Bulldozer> {
+ @Override
+ public List<? extends Machine> get() {
+ Machine machine = new Bulldozer();


There's no need to declare a Machine type variable; you can directly create a list of Bulldozer objects. Consider creating a local variable of type List<Bulldozer> to store the bulldozers and return it.

mateacademy-ai-mentor · 2024-10-08T10:50:08Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ }
+
+ @Override
+ public void fill(List<Object> machines, Machine value) {


The 'fill' method should use List as a parameter instead of List to ensure type safety and avoid potential class cast issues at runtime.

mateacademy-ai-mentor · 2024-10-08T10:50:09Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ public void startWorking(List<? extends Machine> machines) {
+ if (machines.contains(truck)) {
+ truck.doWork();
+ } else if (machines.contains(excavator)) {
+ excavator.doWork();
+ } else if (machines.contains(bulldozer)) {
+ bulldozer.doWork();
+ }


The 'startWorking' method logic is incorrect. It should iterate over the list of machines and call 'doWork()' on each machine. The current implementation checks if the list contains one of the specific machines created as fields, which is not the intended behavior.

mateacademy-ai-mentor · 2024-10-08T10:50:09Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ public void startWorking(List<? extends Machine> machines) {
+ if (machines.contains(truck)) {
+ truck.doWork();
+ } else if (machines.contains(excavator)) {
+ excavator.doWork();
+ } else if (machines.contains(bulldozer)) {
+ bulldozer.doWork();
+ }


The fields 'truck', 'excavator', and 'bulldozer' are not used correctly. They are not needed as fields and should be removed. The 'startWorking' method should work with the passed list of machines.

mateacademy-ai-mentor · 2024-10-08T10:50:09Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ private final Machine bulldozer = new Bulldozer();
+
+ @Override
+ public List<Machine> getAll(Class<? extends Machine> type) {


The 'getAll' method should return a list of the specified type 'T', not the raw 'Machine' type. Adjust the return type and the logic to comply with the generic type 'T'.

mateacademy-ai-mentor · 2024-10-08T10:50:09Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ } else if (Excavator.class.isAssignableFrom(type)) {
+ machines.addAll(excavatorProducer);
+ }
+ return machines;


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.

mateacademy-ai-mentor

Hey there! You've done a great job with the functionality, but we need to ensure our code is safely typed. Let's make sure to use generics properly where indicated, especially with the fill() method and MachineService interface, to maintain type safety. Also, consider adding specific fields to the Truck class to represent its unique attributes. Keep up the good work, and with these small tweaks, your code will be even more robust! 👍🛠️

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-10-08T11:38:12Z

src/main/java/core/mate/academy/model/BulldozerProducer.java

+import core.mate.academy.service.MachineProducer;
+import java.util.List;
+
+public class BulldozerProducer implements MachineProducer<Bulldozer> {


Don't begin class or method implementation with empty line. Remove the empty line at the start of the class definition.

mateacademy-ai-mentor · 2024-10-08T11:38:12Z

src/main/java/core/mate/academy/model/ExcavatorProducer.java

+import core.mate.academy.service.MachineProducer;
+import java.util.List;
+
+public class ExcavatorProducer implements MachineProducer<Excavator> {


Don't begin class or method implementation with an empty line. Remove the empty line at the beginning of the class definition.

mateacademy-ai-mentor · 2024-10-08T11:38:12Z

src/main/java/core/mate/academy/model/Truck.java

 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 {


Don't forget about access modifiers for your custom fields in Truck class. You should define fields with private access modifiers and provide public getters and setters for accessing them.

mateacademy-ai-mentor · 2024-10-08T11:38:12Z

src/main/java/core/mate/academy/model/Truck.java

 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 {


The class Truck should contain custom fields that represent the state of a truck. For example, you might have fields such as loadCapacity, engineType, etc. Make sure to add these fields with appropriate access modifiers and methods (getters/setters).

mateacademy-ai-mentor · 2024-10-08T11:38:12Z

src/main/java/core/mate/academy/model/TruckProducer.java

+import core.mate.academy.service.MachineProducer;
+import java.util.List;
+
+public class TruckProducer implements MachineProducer<Truck> {


Don't begin class or method implementation with an empty line. Remove the empty line before the class declaration.

mateacademy-ai-mentor · 2024-10-08T11:38:13Z

src/main/java/core/mate/academy/service/MachineService.java

@@ -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);


Consider parameterizing the MachineService interface to ensure that the T type is always a subtype of Machine. For example, MachineService<T extends Machine>.

mateacademy-ai-mentor · 2024-10-08T11:38:13Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

 /**
 * Your implementation of MachineService.
 */
-public class MachineServiceImpl {
+public class MachineServiceImpl<T extends Machine> implements MachineService<T> {


Don't begin class or method implementation with empty lines (lines 13-15).

mateacademy-ai-mentor · 2024-10-08T11:38:13Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ public List<T> getAll(Class<? extends T> type) {
+ List<T> machines = new ArrayList<>();
+
+ 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;


Consider creating a local variable in your implementation of getAll(Class type) method to hold the result from the producer's get() method before adding it to the machines list.

mateacademy-ai-mentor · 2024-10-08T11:38:13Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ public void fill(List<Object> machines, Machine value) {
+ machines.replaceAll(ignored -> value);


The fill() method should use generics for its machines parameter to ensure type safety. Please change the parameter type from List<Object> to List<T> where T is a bounded type parameter extending Machine.

mateacademy-ai-mentor · 2024-10-08T11:38:13Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ public void fill(List<Object> machines, Machine value) {
+ machines.replaceAll(ignored -> value);


The size of a list passed to fill() method should remain the same. Just replace the old values with the new ones. This implementation is correct, but please ensure the generic type is used for the method parameter.

mateacademy-ai-mentor

Great job on implementing the BulldozerProducer! 👍 Remember to address the mentioned issues to improve code quality and maintainability. Keep learning and coding!

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-10-08T12:17:04Z

src/main/java/core/mate/academy/model/Bulldozer.java

-/**
- * Add some custom fields that could be only in Bulldozer
- * Do not remove no-args constructor
- */
 public class Bulldozer extends Machine {


Don't begin class or method implementation with an empty line. Please remove the empty line before the class declaration.

mateacademy-ai-mentor · 2024-10-08T12:17:04Z

src/main/java/core/mate/academy/model/Bulldozer.java

-/**
- * Add some custom fields that could be only in Bulldozer
- * Do not remove no-args constructor
- */
 public class Bulldozer extends Machine {


Don't forget about access modifiers for your custom fields in Bulldozer class. It's good practice to use private access modifiers and provide getters and setters for accessing these fields.

mateacademy-ai-mentor · 2024-10-08T12:17:05Z

src/main/java/core/mate/academy/model/Excavator.java

@@ -4,6 +4,7 @@
 * Add some custom fields that could be only in Excavator
 * Do not remove no-args constructor
 */
+
 public class Excavator extends Machine {


Don't begin class implementation with an empty line. According to the checklist, the class definition should not be preceded by an empty line.

mateacademy-ai-mentor · 2024-10-08T12:17:05Z

src/main/java/core/mate/academy/model/ExcavatorProducer.java

+import core.mate.academy.service.MachineProducer;
+import java.util.List;
+
+public class ExcavatorProducer implements MachineProducer<Excavator> {


Don't begin class or method implementation with an empty line.

mateacademy-ai-mentor · 2024-10-08T12:17:05Z

src/main/java/core/mate/academy/model/Truck.java

@@ -4,6 +4,7 @@
 * Add some custom fields that could be only in Truck
 * Do not remove no-field constructor
 */
+
 public class Truck extends Machine {


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.

mateacademy-ai-mentor · 2024-10-08T12:17:06Z

src/main/java/core/mate/academy/service/MachineService.java


+ void fill(List<Object> machines, Machine value);


The fill method should have a bounded wildcard for the machines list to ensure type safety. Use List<? super T> to allow lists of any supertype of T.

mateacademy-ai-mentor · 2024-10-08T12:17:06Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

 /**
 * Your implementation of MachineService.
 */
-public class MachineServiceImpl {
+
+public class MachineServiceImpl<T extends Machine> implements MachineService<T> {


The class 'MachineServiceImpl' should not start with an empty line according to the common mistakes checklist.

mateacademy-ai-mentor · 2024-10-08T12:17:06Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+public class MachineServiceImpl<T extends Machine> implements MachineService<T> {
+ @SuppressWarnings("unchecked")
+ @Override
+ public List<T> getAll(Class<? extends Machine> type) {


Consider creating a local variable for the result of the 'get()' call to avoid casting the list multiple times. This would improve readability and maintainability of the code.

mateacademy-ai-mentor · 2024-10-08T12:17:06Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ }
+
+ @Override
+ public void fill(List<Object> machines, Machine value) {


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.

mateacademy-ai-mentor · 2024-10-08T12:17:06Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ machines.addAll((List<T>) new ExcavatorProducer().get());
+ }
+
+ return machines.isEmpty() ? List.of() : machines;


Keep if-else constructions simple. There is no need for an else clause after a return statement. You can remove the else to flatten the structure of the code.

Elena-Bruyako · 2024-10-08T12:37:11Z

src/main/java/core/mate/academy/service/MachineService.java


+ List<T> getAll(Class<? extends Machine> type);


Suggested change

List<T> getAll(Class<? extends Machine> type);

List<T> getAll(Class<? extends T> type);

Elena-Bruyako · 2024-10-08T12:37:19Z

src/main/java/core/mate/academy/service/MachineService.java


+ void fill(List<Object> machines, Machine value);


Suggested change

void fill(List<Object> machines, Machine value);

void fill(List<? super T> machines, T value);

Elena-Bruyako · 2024-10-08T12:37:45Z

src/main/java/core/mate/academy/service/MachineService.java

 /**
 * 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);


Suggested change

void startWorking(List<? extends Machine> machines);

void startWorking(List<? extends T> machines);

Elena-Bruyako · 2024-10-08T12:38:11Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

 /**
 * Your implementation of MachineService.
 */
-public class MachineServiceImpl {
+
+public class MachineServiceImpl<T extends Machine> implements MachineService<T> {


Suggested change

public class MachineServiceImpl<T extends Machine> implements MachineService<T> {

public class MachineServiceImpl implements MachineService<Machine> {

Elena-Bruyako · 2024-10-08T12:38:56Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ @SuppressWarnings("unchecked")
+ @Override
+ public List<T> getAll(Class<? extends Machine> type) {
+ List<T> machines = new ArrayList<>();


Suggested change

List<T> machines = new ArrayList<>();

List<? extends Machine> machines = null;

if (type == Bulldozer.class) {

machines = bulldozerProducer.get();

}......

Elena-Bruyako · 2024-10-08T12:39:18Z

src/main/java/core/mate/academy/service/MachineServiceImpl.java

+ }
+
+ @Override
+ public void fill(List<Object> machines, Machine value) {


Suggested change

public void fill(List<Object> machines, Machine value) {

public void fill(List<? super Machine> machines, Machine value) {

for (int i = 0; i < machines.size(); i++) {

machines.set(i, value);

}

}

implemented pecs

ff68146

mateacademy-ai-mentor suggested changes Oct 8, 2024

View reviewed changes

fixed the logic of the task

6208a97

Nazar1106 requested a review from mateacademy-ai-mentor October 8, 2024 11:37

mateacademy-ai-mentor suggested changes Oct 8, 2024

View reviewed changes

fixed previous issues

faf9280

Nazar1106 requested a review from mateacademy-ai-mentor October 8, 2024 12:16

mateacademy-ai-mentor approved these changes Oct 8, 2024

View reviewed changes

Elena-Bruyako suggested changes Oct 8, 2024

View reviewed changes

		public void fill(List<Object> machines, Machine value) {
		machines.replaceAll(ignored -> value);

	List<T> getAll(Class<? extends Machine> type);
	List<T> getAll(Class<? extends T> type);

	void fill(List<Object> machines, Machine value);
	void fill(List<? super T> machines, T value);

	void startWorking(List<? extends Machine> machines);
	void startWorking(List<? extends T> machines);

	public class MachineServiceImpl<T extends Machine> implements MachineService<T> {
	public class MachineServiceImpl implements MachineService<Machine> {

- List<T> machines = new ArrayList<>();
+ List<? extends Machine> machines = null;
+ if (type == Bulldozer.class) {
+ machines = bulldozerProducer.get();
+ }......

- public void fill(List<Object> machines, Machine value) {
+ public void fill(List<? super Machine> machines, Machine value) {
+ for (int i = 0; i < machines.size(); i++) {
+ machines.set(i, value);
+ }
+ }

implemented pecs #919

Are you sure you want to change the base?

implemented pecs #919

Conversation

Nazar1106 commented Oct 8, 2024

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment