-
Notifications
You must be signed in to change notification settings - Fork 193
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
[Edly Irsyad] iP #195
base: master
Are you sure you want to change the base?
[Edly Irsyad] iP #195
Conversation
# Conflicts: # src/main/java/Duke.java
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.
Good job! Consider SLAPping more of the code and reduce duplicated code.
|
||
@Override | ||
public String getSymbol() { | ||
return "D"; // mark Deadlines with a "D" |
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.
Consider removing the comment since the code is self-explanatory.
src/main/java/Duke.java
Outdated
} | ||
welcomeMessage(); | ||
ArrayList<Task> entries = new ArrayList<>(); | ||
String dukeDataPath = "C:\\Users\\edly1\\Documents\\ip\\data\\dukeData.txt"; |
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.
If this is a constant, the constant names should be all uppercase using underscore to separate words.
src/main/java/Duke.java
Outdated
case "list": | ||
printLongLine(); | ||
System.out.println("Here are the tasks in your list:"); | ||
int i = 0; | ||
for(Task entry : entries){ | ||
System.out.println((i+1) + ". " + "[" + entry.getSymbol() + "] [" | ||
+ entry.getStatusIcon() + "] " + entry.getDescription()); | ||
i++; | ||
} | ||
printLongLine(); | ||
break; | ||
case "todo": | ||
if (userCommandDetails.equals("todo")) { | ||
System.out.println("Hey! The description of a todo cannot be empty..."); | ||
} | ||
else { | ||
entries.add(entriesCount, new ToDo(userCommandDetails)); | ||
System.out.println("Got it. I've added this task:"); | ||
toBePrinted = "[" + entries.get(entriesCount).getSymbol() + "] [" | ||
+ entries.get(entriesCount).getStatusIcon() + "] " + entries.get(entriesCount).description; | ||
System.out.println(" " + toBePrinted); | ||
entriesCount++; | ||
System.out.println("Now you have " + entriesCount + " tasks in the list."); | ||
try { | ||
writeToFile(dukeDataPath, toBePrinted + System.lineSeparator()); | ||
} catch (IOException e) { | ||
System.out.println("Something went wrong: " + e.getMessage()); | ||
} | ||
} | ||
break; | ||
case "deadline": | ||
int deadlineIndex = userCommandDetails.indexOf("/by"); | ||
String taskDescription = userCommandDetails.substring(0, deadlineIndex - 1); | ||
String taskDeadline = userCommandDetails.substring(deadlineIndex); | ||
entries.add(entriesCount, new Deadline(taskDescription, taskDeadline)); | ||
System.out.println("Got it. I've added this task:"); | ||
toBePrinted = "[" + entries.get(entriesCount).getSymbol() + "] [" | ||
+ entries.get(entriesCount).getStatusIcon() + "] " + entries.get(entriesCount).description; | ||
System.out.println(" " + toBePrinted); | ||
entriesCount++; | ||
System.out.println("Now you have " + entriesCount + " tasks in the list."); | ||
try { | ||
writeToFile(dukeDataPath, toBePrinted + System.lineSeparator()); | ||
} catch (IOException e) { | ||
System.out.println("Something went wrong: " + e.getMessage()); | ||
} | ||
break; | ||
case "event": | ||
int eventDateTimeIndex = userCommandDetails.indexOf("/at"); | ||
String eventDescription = userCommandDetails.substring(0, eventDateTimeIndex - 1); | ||
String eventDateTime = userCommandDetails.substring(eventDateTimeIndex); | ||
entries.add(entriesCount, new Event(eventDescription, eventDateTime)); | ||
System.out.println("Got it. I've added this task:"); | ||
toBePrinted = "[" + entries.get(entriesCount).getSymbol() + "] [" | ||
+ entries.get(entriesCount).getStatusIcon() + "] " + entries.get(entriesCount).description; | ||
System.out.println(" " + toBePrinted); | ||
entriesCount++; | ||
System.out.println("Now you have " + entriesCount + " tasks in the list."); | ||
try { | ||
writeToFile(dukeDataPath, toBePrinted + System.lineSeparator()); | ||
} catch (IOException e) { | ||
System.out.println("Something went wrong: " + e.getMessage()); | ||
} | ||
break; | ||
case "done": | ||
String stringTaskIndex = userMessage.substring(userMessage.indexOf(" ") + 1); | ||
int intTaskIndex = Integer.parseInt(stringTaskIndex) - 1; | ||
entries.get(intTaskIndex).isDone = true; | ||
printLongLine(); | ||
System.out.println("Nice! I've marked this task as done:\n" | ||
+ " [" + entries.get(intTaskIndex).getSymbol() | ||
+ "] [" + entries.get(intTaskIndex).getStatusIcon() + "] " + entries.get(intTaskIndex).description); | ||
printLongLine(); | ||
break; | ||
case "delete": | ||
stringTaskIndex = userMessage.substring(userMessage.indexOf(" ") + 1); | ||
intTaskIndex = Integer.parseInt(stringTaskIndex) - 1; | ||
printLongLine(); | ||
System.out.println("Noted. I've removed this task:\n" | ||
+ " [" + entries.get(intTaskIndex).getSymbol() | ||
+ "] [" + entries.get(intTaskIndex).getStatusIcon() + "] " + entries.get(intTaskIndex).description); | ||
System.out.println("Now you have " + (entriesCount - 1) + " tasks in the list."); | ||
printLongLine(); | ||
for (i = intTaskIndex; i < entriesCount - 1; i++) { | ||
entries.set(i, entries.get(i + 1)); | ||
} | ||
entries.remove(entriesCount - 1); | ||
entriesCount -= 1; | ||
break; | ||
default: | ||
printLongLine(); | ||
System.out.println(userCommand + " is an invalid command, try again"); | ||
printLongLine(); | ||
break; | ||
} | ||
userMessage = in.nextLine(); | ||
} | ||
farewellMessage(); | ||
} | ||
} |
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.
Function is too bulky and long (more than 30 LOC), consider breaking it down into smaller parts and SLAP it further.
src/main/java/Duke.java
Outdated
String userCommand = userMessage.contains(" ") | ||
? userMessage.substring(0, userMessage.indexOf(" ")): userMessage; | ||
String userCommandDetails = userMessage.contains(" ") | ||
? userMessage.substring(userMessage.indexOf(" ") + 1): userMessage; |
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.
Avoid complicated expressions. Consider breaking down the logic into multiple steps.
src/main/java/Duke.java
Outdated
System.out.println("Here are the tasks in your list:"); | ||
int i = 0; | ||
for(Task entry : entries){ | ||
System.out.println((i+1) + ". " + "[" + entry.getSymbol() + "] [" |
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.
Provide adequate spacing for the code (i.e. i + 1 instead)
src/main/java/Duke.java
Outdated
try { | ||
writeToFile(dukeDataPath, toBePrinted + System.lineSeparator()); | ||
} catch (IOException e) { | ||
System.out.println("Something went wrong: " + e.getMessage()); |
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.
Avoid the use of magic strings. In this case, the error message can be extracted into a constant variable with a suitable name.
Also reduces duplicated code.
src/main/java/Duke.java
Outdated
printLongLine(); | ||
System.out.println("Noted. I've removed this task:\n" | ||
+ " [" + entries.get(intTaskIndex).getSymbol() | ||
+ "] [" + entries.get(intTaskIndex).getStatusIcon() + "] " + entries.get(intTaskIndex).description); |
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.
Line is too long (more than 120 characters). Consider separating the line into multiple lines.
Add some paragraph to the README.md to explain ...
Also add a heading ...