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

Hw1 #2145

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

Hw1 #2145

wants to merge 10 commits into from

Conversation

zAdamSz
Copy link

@zAdamSz zAdamSz commented Sep 17, 2024

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Your pull request is almost there! 🌟 However, there are a few important changes needed before we can merge it. The messages in doWork and stopWork methods across all machine classes need to be more informative, indicating the type of machine in action. Also, please adjust the order of doWork and stopWork method calls, and make sure to rename the machine parameter to args in the main method for clarity. Lastly, the Machine class should be declared public. These adjustments are crucial for maintaining clear code functionality and readability. Keep up the good work, and with these changes, your code will be ready to go! 💻🛠️

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

public class Bulldozer extends Machine {
@Override
public void doWork() {
System.out.println("Bulldozer: do 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 specifying the action as well as the type of Machine, for example: 'Bulldozer started its work.'


@Override
public void stopWork() {
System.out.println("Bulldozer: stop work");

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. It should indicate that the Bulldozer has stopped working, for example: 'Bulldozer stopped its work.'

public class Excavator extends Machine {
@Override
public void doWork() {
System.out.println("Excavator: do 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 Excavator is working. For example, it could be "Excavator started its work."


@Override
public void stopWork() {
System.out.println("Excavator: stop work");

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. Consider changing it to something like "Excavator stopped its work." to clearly indicate the action being taken.

Comment on lines 3 to 8
abstract class Machine {

public abstract void doWork();

public abstract void stopWork();
}

Choose a reason for hiding this comment

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

The Machine class should be public to ensure it can be accessed from other packages if needed, which is typically the case for an abstract class that will be subclassed in different packages.

@@ -1,5 +1,19 @@
package core.basesyntax;

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

Choose a reason for hiding this comment

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

The parameter name machine in the main method is misleading since it is an array of String arguments, typically named args. Consider renaming it to args to follow common Java conventions.

Machine bulldozer = new Bulldozer();
Machine excavator = new Excavator();
truck.doWork();
bulldozer.stopWork();

Choose a reason for hiding this comment

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

According to the checklist item 'Write informative messages in methods', the messages should indicate what type of Machine is working. Make sure that the doWork and stopWork methods in Truck, Bulldozer, and Excavator classes print informative messages indicating the machine type.

Machine excavator = new Excavator();
truck.doWork();
bulldozer.stopWork();
excavator.stopWork();

Choose a reason for hiding this comment

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

The order of calling doWork and stopWork seems inconsistent. According to the common practice, a machine should start working before it stops. Please ensure that doWork is called before stopWork for each machine.

public class Truck extends Machine {
@Override
public void doWork() {
System.out.println("Truck: do work");

Choose a reason for hiding this comment

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

The message printed by the doWork method should be more informative. It should indicate that a 'Truck' is working, according to the checklist.


@Override
public void stopWork() {
System.out.println("Truck: stop work");

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 and indicate that a 'Truck' has stopped working.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on following the checklist requirements and ensuring your code adheres to the guidelines! Your use of polymorphism and attention to detail in the class implementations are commendable. Keep up the good work! 🌟 Remember, clear and informative messages in your methods add to the readability and maintainability of your code, so always aim for clarity in your output strings. Keep coding and improving! 👍

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


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

Choose a reason for hiding this comment

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

The message in the stopWork method is not properly formatted. There should be a space after the colon to make the message more readable. Consider changing it to System.out.println("Bulldozer: Bulldozer stopped working"); for consistency with the doWork method.

public class Excavator extends Machine {
@Override
public void doWork() {
System.out.println("Excavator: Excavator started working");

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 indicate the type of machine that is working. Consider including the class name in the message. For example: 'Excavator is starting its work.'


@Override
public void stopWork() {
System.out.println("Excavator: Excavator 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 more informative. Use the class name to indicate which machine stopped working. For example: 'Excavator has stopped working.'

public class Truck extends Machine {
@Override
public void doWork() {
System.out.println("Truck: Truck started working");

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 the type of machine that is working. For example, 'Truck started its work.' would be more informative.


@Override
public void stopWork() {
System.out.println("Truck: Truck 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. Consider rephrasing it to 'Truck has stopped its work.' to clearly indicate the action.

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.

}

Choose a reason for hiding this comment

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

Suggested change

Choose a reason for hiding this comment

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

remove empty line

Machine truck = new Truck();
Machine bulldozer = new Bulldozer();
Machine excavator = new Excavator();
truck.doWork();

Choose a reason for hiding this comment

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

create a Machine array with Truck, Bulldozer, and Excavator and call methods doWork() and stopWork() in a loop.


for (Machine machine : machines) {

Choose a reason for hiding this comment

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

combine into one for loop

@Override
public void doWork() {
System.out.println("Truck: Truck started working");

Choose a reason for hiding this comment

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

remove empty line

}

Choose a reason for hiding this comment

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

remove empty line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants