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

[Teh Kok Hoe] iP #309

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

Conversation

tehkokhoe
Copy link

No description provided.

Change the enum accessor so that it is accessible.
Add error handling to prevent program from terminating due to illegal user input.
Change display to look neater.
Add a semi-automated test to check correctness of code and reduce human error.
Add Exception class to deal with exceptions unique to program.
Add delete feature to remove a task from the list.
Add test cases for delete feature to ensure correct display an exception handling
Add toRecord method for save method to write in a different display from the output on console
Add toRecord method for save method to write in a different display from the output on console
Add save method so that whenever list is changed, it is updated to a text file.
Adjust code to fit coding standard.
Adjust code to fit coding standard
Copy link

@bakano98 bakano98 left a comment

Choose a reason for hiding this comment

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

A few coding standard violations, but other than that, nothing too huge. Remember to put everything into a package.
LGTM after fixing these.

public class Duke {
enum Command {
Copy link

Choose a reason for hiding this comment

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

Good to see you're using enums for this! Keep it up.

@@ -0,0 +1,50 @@
public class Task {
private String task;
private boolean done;
Copy link

Choose a reason for hiding this comment

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

According to the coding standards, this should be named isDone

System.out.println(" Hello! I'm Duke");
System.out.println(" What can I do for you?");
System.out.println(" ____________________________________________________________");
boolean StillIn = true;
Copy link

Choose a reason for hiding this comment

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

Use camelCase to name variables, and booleans should be of the form isSomething. You could possibly rename this to isStillIn

System.out.println(" ____________________________________________________________");
try {
switch (cmd) {
case BYE:
Copy link

Choose a reason for hiding this comment

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

case should not be indented. It should be on the same line as switch

public static void main(String[] args) {
String logo = " ____ _ \n"
+ "| _ \\ _ _| | _____ \n"
+ "| | | | | | | |/ / _ \\\n"
+ "| |_| | |_| | < __/\n"
+ "|____/ \\__,_|_|\\_\\___|\n";
System.out.println("Hello from\n" + logo);
System.out.println(" ____________________________________________________________");
Copy link

Choose a reason for hiding this comment

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

Instead of using 4 spaces, you can consider using \t instead for tab

@@ -1,10 +1,166 @@
import java.util.*;
Copy link

Choose a reason for hiding this comment

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

Two things here:

  1. Keep everything in a package
  2. Only import things that are needed to be used. Avoid using wildcard imports

@@ -0,0 +1,51 @@
import java.io.BufferedReader;
Copy link

Choose a reason for hiding this comment

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

Not too sure if FastReader is required here, but good consideration!

boolean StillIn = true;
FastReader fr = new FastReader();
ArrayList<Task> list = new ArrayList<Task>(100);
while(StillIn) {
Copy link

Choose a reason for hiding this comment

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

I think you need to put spacing between the brackets and each while and each if. You only do not use spaces when it is a function/method declaration

Allow dates to be recognized as 'Date' objects to allow more capabilities like sorting around dates
# Conflicts:
#	src/main/java/Duke.java
Adjust how date works due to complement save implementation
Seperate operations into different classes that do specific things
Fix how date is parsed in event so that it can read the two dates regardless of whitespace between the seperator '-' and the date
Package all classes into one package
Add exception handler for invalid commands by telling user command is invalid and then waits to receive next user input
tehkokhoe and others added 25 commits February 25, 2022 10:06
Let parseCommand throw IllegalArgumentException when receive invalid user input
Add JUnit tests for invalid commands and check if bye command quits program
Add checkstyle tests to ensure coding standard
Add JavaDoc to allow people to use program easily
Fix coding standards for improved readability
Add find function so users can easily filter tasks with specific keywords
# Conflicts:
#	src/main/java/duke/FastReader.java
#	src/main/java/duke/UI.java
Add JavaDoc for find methods for developer clarity
Add GUI so it is more accessible
Adjust JavaDoc for GUI
Curent code might not be able to detect some bugs

So that major bugs can be detected when code is changed in the future

Add some asserts to ensure that responses should not be empty for most commands

Assert is low cost and is worth the extra security
Users can add duplicate tasks into the list

Using equals and contain methods allows duplicate checking

Easy to implement and understand
Change the user picture and input text to be always at the bottom
of the dialog box, and constantly refreshes as users shouldn't
have to know what input they have previously typed, and can infer
by just looking at Duke's output
Add User Guide for users to use program comprehensively
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.

2 participants