-
Notifications
You must be signed in to change notification settings - Fork 187
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
[Ranj Nicoli M. Negapatan] iP #177
base: master
Are you sure you want to change the base?
Conversation
Branch level 5
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.
Generally oki job but some coding standard and quality to follow! do avoid long methods 👍
src/main/java/Nocturne.java
Outdated
|
||
public class Nocturne { | ||
public static void main(String[] args) throws NocturneException { | ||
Task[] tasks = new Task[100]; |
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.
just a recommendation: perhaps you might want to put the number 100 as a named constant to avoid magic numbers! it will also be easier for you to find & change this number in the future if everything is put on top :]
src/main/java/Nocturne.java
Outdated
|
||
input = in.nextLine(); | ||
while (!input.equals("bye")) { | ||
String[] commandCheck = input.split(" "); |
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.
note that plural form should be used on names representing a collection of objects. might want to change this to a different name instead to for better readability - perhaps something like commandParts/commandTypes or other appropriate names!
src/main/java/Nocturne.java
Outdated
switch (commandCheck[0]) { | ||
case "list": | ||
if (taskCount == 0) { | ||
throw new NocturneException("An empty list begets an empty mind."); | ||
} | ||
for (int i = 0; i < taskCount; i++) { | ||
System.out.println((i + 1) + "." + tasks[i]); | ||
} | ||
break; | ||
|
||
case "mark": { | ||
if (commandCheck.length < 2 | commandCheck.length > taskCount + 1) { | ||
throw new NocturneException("Your list is either empty or your brain is."); | ||
} | ||
int listIndex = Integer.parseInt(commandCheck[1]); | ||
System.out.println("Congratulations. I have marked this task as finished:"); | ||
tasks[listIndex - 1].isDone = true; | ||
System.out.println(" " + tasks[listIndex - 1]); | ||
break; | ||
} | ||
case "unmark": { | ||
if (commandCheck.length < 2 | commandCheck.length > taskCount + 1) { | ||
throw new NocturneException("Your list is either empty or your brain is."); | ||
} | ||
int listIndex = Integer.parseInt(commandCheck[1]); | ||
System.out.println("Do not neglect your duties. I have marked this task as unfinished:"); | ||
tasks[listIndex - 1].isDone = false; | ||
System.out.println(" " + tasks[listIndex - 1]); | ||
break; | ||
} | ||
case "deadline": | ||
String[] deadlineSeparated = input.split("/"); | ||
if (deadlineSeparated.length != 3) { | ||
throw new NocturneException("Take your / back, and only put 2!"); | ||
} | ||
Deadline trueDeadline; | ||
String deadlineName = deadlineSeparated[0].substring(9); | ||
String by = deadlineSeparated[1].substring(3); | ||
trueDeadline = new Deadline(deadlineName, by); | ||
tasks[taskCount] = trueDeadline; | ||
taskCount++; | ||
System.out.println("A deadline I see. I have added it:"); | ||
System.out.println(" " + trueDeadline); | ||
break; | ||
|
||
case "todo": | ||
String todoName = input.substring(5); | ||
Todo trueTodo = new Todo(todoName); | ||
tasks[taskCount] = trueTodo; | ||
taskCount++; | ||
System.out.println("A Todo task I see. I have added it:"); | ||
System.out.println(" " + trueTodo); | ||
break; | ||
|
||
case "event": | ||
String[] eventSeparated = input.split("/"); | ||
if (eventSeparated.length != 3) { | ||
throw new NocturneException("Take your / back, and only put 2!"); | ||
} | ||
Event trueEvent; | ||
String eventName = eventSeparated[0].substring(6); | ||
String from = eventSeparated[1].trim(); | ||
from = from.substring(5); | ||
String to = eventSeparated[2].substring(3); | ||
trueEvent = new Event(eventName, from, to); | ||
tasks[taskCount] = trueEvent; | ||
taskCount++; | ||
System.out.println("An event I see. I have added it:"); | ||
System.out.println(" " + trueEvent); | ||
break; | ||
|
||
default: | ||
throw new NocturneException("Your command is invalid. Try again."); | ||
} |
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.
switch statement should be in the following form:
switch (condition) {
case ABC:
statements;
// Fallthrough
case DEF:
statements;
break;
case XYZ:
statements;
break;
default:
statements;
break;
}
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.
additionally, might want to consider moving & refactoring the different operations to different methods (e.g. listTasks(), markTask()) so that everything is not under the main method! try to avoid long methods :]
src/main/java/Nocturne.java
Outdated
break; | ||
} | ||
case "deadline": | ||
String[] deadlineSeparated = input.split("/"); |
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.
Plural form should be used on names representing a collection of objects
src/main/java/Nocturne.java
Outdated
break; | ||
|
||
case "event": | ||
String[] eventSeparated = input.split("/"); |
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.
Plural form should be used on names representing a collection of objects.
Increment to Level 7
branch-Level-9
A-UserGuide
No description provided.