[AY1920S1-CS2113T-F10-2] Duke 2.0 #36
Conversation
* B-Reminders * B-reminders modification * duke.java * modified buidl.gradle and Command * user guide * user guide table of contents * user guide link TOC * user guide * UI * corrected mock UI * rename UI * UI update
There was a problem hiding this comment.
Nice job! The code is quite clean and well done. Also good job on including test code. Do try to follow the commenting guidelines and not include too much information in your comments. Also remember that comments for methods should be in simple present tense.
Some code looks like it is repeated many times. See if you can apply some abstraction techniques.
Please close this pr after you have read the comments.
| private String arguments; | ||
| private String command; | ||
| private Memento memento; | ||
| private int listType = 0; //0 for task list, 1 for degree list |
| * @since 09/19 | ||
| */ | ||
| public abstract class Command { | ||
| boolean exit = false; |
| import list.DegreeList; | ||
|
|
||
| /** | ||
| * AddCommand Class extends the abstract Command class. |
There was a problem hiding this comment.
Comments should focus on what the code is supposed to do. You should not include excess information that may be redundant. (extends can be seen from AddCommand extends Command)
| TaskList tasksBuffer; | ||
| DegreeList degreesBuffer; | ||
|
|
||
| if (this.command.matches("event")) { |
There was a problem hiding this comment.
This code seems repetitive. Is there a way to abstract?
| } | ||
|
|
||
| /** | ||
| * overwrites default execute to add tasks. |
There was a problem hiding this comment.
Describe what the method does. You can use @OverRide to show the override instead of writing in the comment description.
| * @param mc is the credit amount of a module | ||
| */ | ||
| public Module(String code, String name, Integer mc) | ||
| { |
| try (BufferedWriter bufferedWriter = new BufferedWriter(new FileWriter(this.saveFile))) { | ||
| for (int i = 0; i < list.size(); i++) { | ||
| String fileContent = list.get(i).getType() + " | " | ||
| + (list.get(i).checkCompletion() ? "1" : "0") + " | " + list.get(i).getDescription(); |
There was a problem hiding this comment.
The statement is quite complex, can it be simplified to be more understandable with refactoring?
| for (int i = 0; i < splitTasks.length; i++) { | ||
| String[] split = splitTasks[i].split(Parser.taskSeparator); | ||
| switch (split[0]) { | ||
| case "T": |
src/main/java/task/After.java
Outdated
| */ | ||
| public After(String description) throws DukeException { | ||
| String[] split = description.split(Parser.after); | ||
| if (split.length < 2) { |
There was a problem hiding this comment.
This code seems to be repeated in many classes is there a way to abstract?
| class ExitCommandTest { | ||
|
|
||
| private Command testCommand = new ExitCommand(); | ||
| private TaskList testTaskList = new TaskList("T | 0 | Send even more Help\n" |
There was a problem hiding this comment.
If you use this for a lot of tests you may want to abstract it out as a test utility
… into AY1920S1-CS2113T-F10-2-master
… into AY1920S1-CS2113T-F10-2-master # Conflicts: # classes/java/main/degree/Degree.class # classes/java/main/module/Module.class # classes/java/main/module/ModuleList.class # src/main/java/degree/Degree.java # src/main/java/degree/DegreeManager.java # src/main/java/module/ConjunctiveModule.java # src/main/java/module/ModuleList.java
Added all tables
* 'master' of https://github.com/manaswini05/main: Finish All 4 tables Merge branch 'master' of https://github.com/AY1920S1-CS2113T-F10-2/main into AY1920S1-CS2113T-F10-2-master fix gui resize bug prep for diff and similar checking prep for diff checking fix minor bugs udpate javadoc finish tables for all lists fix autosuggest recording typos prep for degree information add table view for task and choices add table support for task
build new jar file
fix removeDegreeTasks in UniversityTaskHandler.java
implement support for saving taskPriority
remove support for After, Fixed, Recurring, and Within Tasks
add photos
* Remove instanceof from ReminderCommand * Generalize reminders to tasks
@manaswini05 @zyleet @KaneQuah @woblek