-
Notifications
You must be signed in to change notification settings - Fork 590
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
[Haris-Irfan] iP #618
base: master
Are you sure you want to change the base?
[Haris-Irfan] iP #618
Conversation
In build.gradle, the dependencies on distZip and/or distTar causes the shadowJar task to generate a second JAR file for which the mainClass.set("seedu.duke.Duke") does not take effect. Hence, this additional JAR file cannot be run. For this product, there is no need to generate a second JAR file to begin with. Let's remove this dependency from the build.gradle to prevent the shadowJar task from generating the extra JAR file.
src/main/java/KieTwoForOne.java
Outdated
public static boolean isCompleteInput(String[] input) throws KieTwoForOneException { | ||
if (input.length <= 1 && !input[0].equalsIgnoreCase("list") && !input[0]. equalsIgnoreCase("bye")) { | ||
throw new KieTwoForOneException(); | ||
} | ||
return true; | ||
} | ||
|
||
public static boolean isCompleteEventInput(String[] input) throws KieTwoForOneException { | ||
if (input.length != 3) { | ||
throw new KieTwoForOneException(); | ||
} | ||
return true; | ||
} | ||
|
||
public static boolean isCompleteDeadlineInput(String[] input) throws KieTwoForOneException { | ||
if (input.length != 2) { | ||
throw new KieTwoForOneException(); | ||
} |
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.
Perhaps could change to
isInputComplete, isEventInputComplete, and isDeadlineInputComplete
to sound more explicit
src/main/java/KieTwoForOne.java
Outdated
|
||
public class KieTwoForOne { | ||
|
||
private static ArrayList<Task> tasks = new ArrayList<>(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.
The number 100 is a magic number.
Consider defining it as a constant with a meaningful name, such as MAX_TASKS?
src/main/java/KieTwoForOne.java
Outdated
switch (Instructions.valueOf(instruction[0].toUpperCase())) { | ||
case LIST: | ||
KieTwoForOne.listTasks(); | ||
break; | ||
case BYE: | ||
System.out.println("Bye. Hope to see you again soon!"); | ||
System.out.println(separationLine); | ||
break; | ||
case MARK: | ||
try { | ||
KieTwoForOne.markTask(Integer.valueOf(instruction[1])); | ||
} catch (IndexOutOfBoundsException e) { | ||
System.out.println("Task does not exist!"); | ||
System.out.println(separationLine); | ||
break; | ||
} | ||
break; | ||
case UNMARK: | ||
try { | ||
KieTwoForOne.unmarkTask(Integer.valueOf(instruction[1])); | ||
} catch (IndexOutOfBoundsException e) { | ||
System.out.println("Task does not exist!"); | ||
System.out.println(separationLine); | ||
break; | ||
} | ||
break; | ||
case TODO: | ||
KieTwoForOne.addTasks(new Todo(instruction[1])); | ||
break; | ||
case EVENT: | ||
try { | ||
isCompleteEventInput(taskDetails); | ||
} catch (KieTwoForOneException e){ | ||
System.out.println("Please input a start and end time!"); | ||
System.out.println(separationLine); | ||
break; | ||
} | ||
KieTwoForOne.addTasks(new Event(taskDetails[0], taskDetails[1], taskDetails[2])); | ||
break; | ||
case DEADLINE: | ||
try { | ||
isCompleteDeadlineInput(taskDetails); | ||
} catch (KieTwoForOneException e) { | ||
System.out.println("Please input a deadline!"); | ||
System.out.println(separationLine); | ||
break; | ||
} | ||
KieTwoForOne.addTasks(new Deadline(taskDetails[0], taskDetails[1])); | ||
break; | ||
case DELETE: | ||
try { | ||
deleteTask(Integer.valueOf(instruction[1])); | ||
} catch (IndexOutOfBoundsException e) { | ||
System.out.println("Task does not exist!"); | ||
System.out.println(separationLine); | ||
break; | ||
} | ||
break; | ||
default: | ||
break; | ||
} | ||
if (instruction[0].equalsIgnoreCase("bye")) { | ||
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.
Fix styling to align with styleguide?
src/main/java/Duke.java
Outdated
|
||
while(true) { | ||
String instruction = scanner.nextLine(); | ||
switch (instruction.toUpperCase()) { |
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.
Shouldn't there be no indentation for case
clauses?
src/main/java/KieTwoForOne.java
Outdated
|
||
while(true) { | ||
String instruction = scanner.nextLine(); | ||
switch (instruction.toUpperCase()) { |
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.
Shouldn't there be no indentation for case
clauses?
src/main/java/KieTwoForOne.java
Outdated
private static int count = 0; | ||
static String separationLine = "_________________________________________"; | ||
static String chatBotName = "KieTwoForOne"; | ||
|
||
public static void addTasks(String task) { | ||
tasks[count] = task; | ||
public enum Instructions { |
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.
I like how you use enum
to represent all the instructions.
src/main/java/KieTwoForOne.java
Outdated
String input = scanner.nextLine().toUpperCase(); | ||
String[] instruction = input.split(" ", 2); | ||
|
||
switch (Instructions.valueOf(instruction[0])) { |
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.
Shouldn't there be no indentation for case
clauses?
Include the exception thrown in the default case of the switch statement in the parse method. Ensures that by default any invalid inputs will display an "Invalid input" in the chatbot. Include missing Java Documentation for some methods
Implement assertions Implement assertions to ensure certain assumptions in different methods are true.
Improve code quality Include the exception thrown in the default case of the switch statement in the parse method. Ensures that by default any invalid inputs will display an "Invalid input" in the chatbot. Include missing Java Documentation for some methods
Tagging functionality allows users to tag their tasks with certain keywords such as #work and #play to better categorise the tasks in their list Done by inputting tag (index) /(tag) in the chatbot
Fixed issue with tagging functionality where invalid inputs were not being handled properly. Fixed issue with GUI where data from the tasks.txt file was not being loaded into the chatbot. Fixed issue where adding a deadline did not include the time of the deadline of the task.
Creates the file path needed to store task data when the user first opens the chatbot.
Included a screenshot of the application with data population (listing of different tasks currently added within the task list) A brief introduction to the chatbot and an in-depth guide to the different features that users can use.
The tagging method only tagged the task temporarily. Needed it such that tag was a permanent addition to the task
Edited JavaDoc comments so that they are in line with coding standards.
Description is not accurate or appealing to users Include more syntax styling and emojis Include more information on features
User guide for downloading the application not provided in README Include header for a guide to download the chatbot
User guide for downloading the application not provided in README Include short description on where users can find the JAR file and how to start the chatbot
User guide for downloading the application not provided in README Include a fenced code block to guide users on how to open JAR file
User guide for downloading the application not provided in README Edit description to provide users more clarity on where they can find the release and how to run the JAR file
README not formatted clearly Remove "Its features are listed below" Following section is not features
README not formatted clearly Add text for users to know where the features can be found Helps users find the features section
README not visually appealling Add italics to make README more attractive to users Adds variance to the text
README not visually appealing Include bold words to make the README more attractive to users Adds variance to the text
README not formatted clearly Add header for introduction Helps users identify the introduction
No description provided.