Skip to content
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

Develop #2155

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Develop #2155

Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/main/java/core/basesyntax/Bulldozer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package core.basesyntax;

public class Bulldozer extends Machine {

public Bulldozer(String machineType) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor is taking a parameter 'machineType' that seems unnecessary. The type of machine is already known (Bulldozer) and does not need to be passed as a parameter.

super(machineType);
}

@Override
public void doWork() {
System.out.println("Bulldozer" + " " + machineType + " " + "started its work.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message in the doWork method should be more informative and indicate that the Bulldozer is working. Consider including the specific action it performs, for example, 'Bulldozer is moving earth'.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'machineType' field is not defined in this class. If 'machineType' is a field in the parent class, it should be accessed using 'super.machineType'. Or better yet, since this class already represents a specific machine type (Bulldozer), 'machineType' is not needed. Simply print "Bulldozer started its work."

}

@Override
public void stopWork() {
System.out.println("Bulldozer" + " " + machineType + " " + "stopped working.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, the message in the stopWork method should be more informative. Rather than just saying 'stopped working', it could be more descriptive, like 'Bulldozer has finished moving earth'.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'machineType' field is not defined in this class. If 'machineType' is a field in the parent class, it should be accessed using 'super.machineType'. Or better yet, since this class already represents a specific machine type (Bulldozer), 'machineType' is not needed. Simply print "Bulldozer stopped working."

}

@Override
public String toString() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'machineType' field is not defined in this class. If 'machineType' is a field in the parent class, it should be accessed using 'super.machineType'. Or better yet, since this class already represents a specific machine type (Bulldozer), 'machineType' is not needed in the toString() method. You can return "Bulldozer" instead.

return machineType;
}
}
24 changes: 24 additions & 0 deletions src/main/java/core/basesyntax/Excavator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package core.basesyntax;

public class Excavator extends Machine {

public Excavator(String machineType) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor for the Excavator class is unnecessary as it just calls the superclass constructor. Consider removing it if there's no additional logic inside.

super(machineType);
}

@Override
public void doWork() {
System.out.println("Exavator" + " " + machineType + " " + "started its work.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a typo in the message printed to the console. It should be 'Excavator' instead of 'Exavator'. Make sure the messages are informative and correctly spelled.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a typo in 'Excavator'. Please correct it.

}

@Override
public void stopWork() {
System.out.println("Exavator" + " " + machineType + " " + "stopped working.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same typo is present in the 'stopWork' method. It should be corrected to 'Excavator'.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a typo in 'Excavator'. Please correct it.


}

@Override
public String toString() {
return machineType;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear what the purpose of the overridden toString() method is, as it only returns 'machineType'. If it's not necessary, consider removing it to use the inherited method from Object class.

}
}
19 changes: 19 additions & 0 deletions src/main/java/core/basesyntax/Machine.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package core.basesyntax;

public abstract class Machine {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how this class should look like:

public abstract class Machine {
    public abstract void doWork();

    public abstract void stopWork();
}

protected String machineType;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'machineType' variable seems unnecessary in this context as the type of machine can be determined by the instance of the class. Consider removing this and the associated constructor and toString method if not needed for other parts of your program.


public Machine(String machineType) {
this.machineType = machineType;
}

public abstract void doWork();

public abstract void stopWork();

@Override
public String toString() {
return machineType;
}
}

19 changes: 19 additions & 0 deletions src/main/java/core/basesyntax/MainApp.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
package core.basesyntax;

import java.util.Arrays;

public class MainApp {
public static void main(String[] args) {

Machine truck = new Truck("Mikki");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructors for Truck, Bulldozer, and Excavator classes are not defined in the task. If these classes have constructors that take parameters, mention it in the task definition. If not, use default constructors instead.

Machine bulldozer = new Bulldozer("Eduardo");
Machine excavator = new Excavator("Maks");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Mateusz mentioned in the previous comment, you should remove all constructors from Machine child classes. If Java class doesn't have any explicitly defined constructor it will use the default one, so in this case you will be able to create Truck/Bulldozer/Excavator instances like this: Machine truck = new Truck(); Notice that in this case you don't have to provide "name" of the machine, because you are able to use default constructor, i.e. new Truck(); That's why you should remove "name" fields in all child classes and delete a constructors you created.


Machine[] machines = {truck, bulldozer, excavator};
printTotal(machines);
System.out.println();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a redundant empty line after the call to printTotal(machines) method. According to the checklist, we should avoid adding unnecessary empty lines after method calls.

}

private static void printTotal(Machine[] machines) {
for (Machine machine : machines) {
machine.doWork();
machine.stopWork();
}
System.out.println(Arrays.toString(machines));
MrGizmen marked this conversation as resolved.
Show resolved Hide resolved

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The System.out.println(Arrays.toString(machines)); does not provide an informative message about the machines. It's better to have messages that indicate what type of machine is working. Consider overriding the toString method in each Machine subclass to provide more informative output.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is unnecessary. The printTotal method is supposed to invoke doWork() and stopWork() on each machine, there is no need to print an array of Machines.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, should only output this?

Truck Mikki started its work.
Truck Mikki stopped working.
Bulldozer Eduardo started its work.
Bulldozer Eduardo stopped working.
Excavator Maks started its work.
Excavator Maks stopped working.
image

}
}
24 changes: 24 additions & 0 deletions src/main/java/core/basesyntax/Truck.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package core.basesyntax;

public class Truck extends Machine {

public Truck(String machineType) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Machine class was not described as having a constructor that takes a machineType parameter. It's better to remove the constructor from the Truck class.

super(machineType);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove constructor and "name" field to be able to create instances without providing a name


@Override
public void doWork() {
System.out.println("Truck" + " " + machineType + " " + "started its work.");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message should be more informative and in proper English. Instead of concatenating strings with spaces, you can use a formatted string. For example: System.out.println("Truck " + machineType + " started its work.");

}

@Override
public void stopWork() {
System.out.println("Truck" + " " + machineType + " " + "stopped working. ");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stop message should also be informative and properly formatted. Consider changing it to: System.out.println("Truck " + machineType + " stopped working.");


}

@Override
public String toString() {
return machineType;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The toString method should return a string that represents the object. Returning only machineType is not very informative. Consider including the class name and other relevant information.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The toString() method is generally used to provide a string representation of the object. In this case, it would be more appropriate to include additional information, such as the class name.

}
}
Loading