-
Notifications
You must be signed in to change notification settings - Fork 364
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
[gitsac] iP #180
base: master
Are you sure you want to change the base?
[gitsac] iP #180
Conversation
…k output when using list command
text output of certain messages
…getting class fields
# Conflicts: # src/main/java/Duke.java # src/main/java/duke/Parser.java # src/main/java/duke/TaskList.java # src/main/java/duke/Ui.java # src/main/java/duke/tasktypes/Deadlines.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.
Overall, I can see that you have put in a lot of efforts to verify inputs. Just need to address some small changes to your code. Keep up the good work! 👍
src/main/java/data/dukedata.txt
Outdated
1.[T][X] test2 | ||
2.[T][X] foru | ||
3.[D][X] forum enjoyer (by: Tuesday) |
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.
You seem to have 2 data text file and folder. Just a reminder to double check you file path and delete the redundant folder
if (printThisOut.equals("Wrong size for mark/unmark")) { | ||
toReturn = ":( Sorry, the number input is wrong. Please check the possible indexes again using list!"; | ||
} | ||
if (printThisOut.equals("deadline") || printThisOut.equals("todo")) { | ||
toReturn = ":( Sorry, the description of a " + this.printThisOut + " cannot be empty!"; | ||
} | ||
if (printThisOut.equals("event")) { | ||
toReturn = ":( Sorry, the description of an " + this.printThisOut + " cannot be empty!"; | ||
} | ||
if (printThisOut.equals("find")) { | ||
toReturn = ":( Sorry, please input a keyword!"; | ||
} |
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.
Possible to use a switch statement here with fall through for deadline and todo but this is up to you.
src/main/java/duke/Parser.java
Outdated
if (input.startsWith("todo")) { | ||
handleToDoTask(input, listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.startsWith("find")) { | ||
handleFindTask(input, listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.startsWith("deadline")) { | ||
handleDeadlineTask(input, listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.startsWith("event")) { | ||
handleEventTask(input, listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.equals("list")) { | ||
handleList(listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.startsWith("delete")) { | ||
handleDelete(input, listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.startsWith("checkdue")) { | ||
handleCheckDue(input, listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.startsWith("mark")) { | ||
handleMark(input, listOfTasks); | ||
return; | ||
} | ||
|
||
if (input.startsWith("unmark")) { | ||
handleUnmark(input, listOfTasks); | ||
return; | ||
} |
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.
One way to use 'startsWith' together with a switch statement is to use the split method.
src/main/java/duke/Ui.java
Outdated
package duke; | ||
|
||
import java.io.BufferedReader; | ||
import java.io.InputStreamReader; | ||
import java.io.IOException; | ||
|
||
/** | ||
* Class used to read user's input for Duke chatbot. | ||
*/ | ||
public class Ui { | ||
|
||
protected BufferedReader readingInput; | ||
|
||
/** | ||
* Constructor to initiate a bufferedreader to get user input. | ||
*/ | ||
public Ui() { | ||
this.readingInput = new BufferedReader(new InputStreamReader(System.in)); | ||
} | ||
|
||
/** | ||
* Function to read user input. | ||
* @return String representation of user input. | ||
* @throws IOException | ||
*/ | ||
public String gettingUserInput() throws IOException { | ||
String userInput = readingInput.readLine(); | ||
return userInput; | ||
} | ||
|
||
} |
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 all you printing of messages could also be handled by the UI class
/** | ||
* Class which represents a Task with a deadline. | ||
*/ | ||
public class Deadlines extends Task { |
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 it is better to name is in singular form as each object from this class is a singular deadline.
* @return String representation of deadline task. | ||
*/ | ||
@Override | ||
public String toString() { |
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.
Try to keep methods to a limit of 30 lines if possible
*/ | ||
public class DukeExceptions extends Exception { | ||
protected String printThisOut; | ||
|
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.
Better to use a variable name like toPrint
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.
Also better to call your exception DukeException
import duke.tasktypes.Events; | ||
import duke.tasktypes.ToDo; | ||
import duke.tasktypes.Task; | ||
|
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.
Great use of packages!
scannerForFileData.close(); | ||
return useThis; | ||
} | ||
|
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.
Might need to consider corrupted files and handle them gracefully
toPrint = "Got it. I've added this task:\n " + toAdd.toString() + "\nNow you have " + listOfTasks.size() + " tasks in the list."; | ||
} | ||
System.out.println(toPrint); | ||
} |
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.
Printing listOfTasks.size() should be abstracted out to a function. Now there is a lot of repeated code. The function could also take in a boolean parameter isPlural for that extra "s" case
return this.name; | ||
} | ||
|
||
} |
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.
Task should be responsible for printing the [X] as this is repeated across all subclasses
In many methods, it is assumed that certain variables or objects are of a certain state. By using assertions, code quality can be improved to make those assumptions a guarantee. Assumptions make it difficult to debug if the error stack trace is very long. It is important to hold assumptions in current state as otherwise there will be a need to check for many things at all methods. Let's improve code quality and make debugging easier by adding assumptions.
The code body is functioning well but makes for poor reading at times due to issues like big chunks of code and deep nesting. By abstracting deep nesting and big chunks of code into multiple helper functions, the code becomes more readable. Let's practice SLAP and be mindful of readability in the future.
Add assertions to Duke
Improve code quality by refactoring.
Knight
"Now, your mind can forget what's to be done, for I will remember it for you." - Knight
Knight frees you from life's smallest worries, by remembering all the tasks you need to do. It's:
Setting you up for success
Ready to remember less things, and start doing more?
All you need to do is,
java -jar Duke.jar
And it is FREE!
Features:
If you happen to do a little coding in Java, see if this makes sense to you: