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

[AdiMangalam] ip #111

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

Conversation

AdiMangalam
Copy link

No description provided.

Copy link

@lucas-sc0 lucas-sc0 left a comment

Choose a reason for hiding this comment

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

Overall great job on code quality!

}

public static void event(String input) {
String[] parts = input.substring(6).split(" /from | /to");

Choose a reason for hiding this comment

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

Avoid using magic numbers like substring(6).split. Used named constants for these numbers instead.

Copy link

@kennethszj kennethszj left a comment

Choose a reason for hiding this comment

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

well done. no noticeable coding standard violation

@@ -0,0 +1,107 @@
import java.sql.Array;
import java.util.Scanner;

Choose a reason for hiding this comment

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

well done. code functionality is implemented with well organized methods and with naming that describes the purpose well, and is in camelCase. if else statement is also in egyptian format

@@ -0,0 +1,31 @@
public class Task {
protected String description;
protected boolean isDone;

Choose a reason for hiding this comment

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

well done. code has proper and regular formatting and is easily understood

public class Deadline extends Task{
protected String by;

public Deadline(String description, String by) {

Choose a reason for hiding this comment

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

has proper use of inheritance and no noticeable formatting issues.


@Override
public String toString() {
return "[D]" + super.toString() + "(by: " + by + ")" ;

Choose a reason for hiding this comment

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

proper use of overiding and with clear comments

this.from = from;
this.to = to;
}

Choose a reason for hiding this comment

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

proper use of overiding and with clear comments

@thoriritu
Copy link

Overall, function names and variables have been appropriately named, abstraction is properly shown. You could work on the documentation of code through the use of comments, so that it is more readable.

Good job!

import java.util.List;
import java.util.ArrayList;

public class Flash {

Choose a reason for hiding this comment

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

To improve abstraction, you can refactor the code by separating task-related logic from the main control flow. One class can handle the application's control logic (user interaction, commands) while another class can be responsible for managing tasks (adding, deleting, marking, saving, etc.).

System.out.println("Nice! I've marked this task as done:");
System.out.println(" " + task);
System.out.println("____________________________________________________________");
} catch (Exception e) {

Choose a reason for hiding this comment

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

Add a specific check for missing task numbers to improve the error handling instead of generic exception.

System.out.println("____________________________________________________________");
}

public static void markTask(String input) throws FlashException {

Choose a reason for hiding this comment

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

The logic for marking and unmarking tasks is almost identical. You can refactor this into a single method to avoid duplication.

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.

4 participants