[Yen Pin Hsuan] iP #97
Conversation
# Conflicts: # src/main/java/Duke.java
# Conflicts: # src/main/java/duke/Duke.java # src/main/java/duke/Storage.java # src/main/java/duke/task/Deadline.java # src/main/java/duke/task/Event.java # src/main/java/duke/task/Task.java # src/main/java/duke/task/ToDo.java
# Conflicts: # src/main/java/duke/Parser.java # src/main/java/duke/TaskList.java # src/main/java/duke/task/Task.java
src/main/java/duke/Parser.java
Outdated
| public static ToDo parseTodo(String input) throws DukeException { | ||
| if (input.equals("")) { | ||
| throw new DukeException("Oops! Todo cannot be empty"); |
There was a problem hiding this comment.
I'm a bit confused here... maybe can change the variable "input" to "detail" like in the parse method because for some reason i assumed the input is the whole string ><
src/main/java/duke/Parser.java
Outdated
| if (arr.length == 1 || arr[0].trim().equals("")) { | ||
| throw new DukeException("Oops! You need to include both detail and time."); | ||
| } | ||
| String detail = arr[0].trim(); |
There was a problem hiding this comment.
I think maybe can factor our the arr[0].trim() a bit earlier here?
src/main/java/duke/Parser.java
Outdated
| case BYE: | ||
| return new ByeCommand(); | ||
| case LIST: | ||
| return new ListCommand(); | ||
| case DONE: | ||
| return new DoneCommand(parseNumber(detail)); | ||
| case TODO: | ||
| return new TodoCommand(parseTodo(detail)); | ||
| case EVENT: | ||
| return new EventCommand(parseEvent(detail)); | ||
| case DEADLINE: | ||
| return new DeadlineCommand(parseDeadline(detail)); | ||
| case DELETE: | ||
| return new DeleteCommand(parseNumber(detail)); | ||
| case FIND: | ||
| return new FindCommand(detail.trim()); | ||
| default: | ||
| throw new DukeException("Oops! I'm sorry, but I don't know what that means"); |
There was a problem hiding this comment.
I like this part!!!! So neat! Can I learn from you? :")
src/main/java/duke/Storage.java
Outdated
| while (buffReader.ready()) { | ||
| String savedTask = buffReader.readLine(); | ||
| String type = savedTask.substring(0, 1); | ||
| switch (type) { | ||
| case "T": | ||
| list.add(new ToDo(savedTask.substring(4).trim(), savedTask.substring(2, 3).equals("T"))); | ||
| break; | ||
| case "D": | ||
| String deadlineDetails = savedTask.substring(4); | ||
| String[] deadlineArr = deadlineDetails.split("/by"); | ||
| Deadline deadline = new Deadline( | ||
| deadlineArr[0].trim(), | ||
| savedTask.substring(2, 3).equals("T"), | ||
| LocalDateTime.parse(deadlineArr[1].trim())); | ||
| list.add(deadline); | ||
| break; | ||
| case "E": | ||
| String eventDetails = savedTask.substring(4); | ||
| String[] eventArr = eventDetails.split("/at"); | ||
| Event event = new Event( | ||
| eventArr[0].trim(), | ||
| savedTask.substring(2, 3).equals("T"), | ||
| LocalDateTime.parse(eventArr[1].trim())); | ||
| list.add(event); |
There was a problem hiding this comment.
Maybe this part can be abstracted into another method? like 1 method to read the file and another to convert the read data to actual objects.
src/main/java/duke/TaskList.java
Outdated
| List<Task> filteredList = list.stream() | ||
| .filter(t -> t.getDetails().contains(keyword)) |
There was a problem hiding this comment.
This is so clever! I used for loop guess now I have to change 👍
src/main/java/duke/TaskList.java
Outdated
| while (iterator.hasNext()) { | ||
| count++; | ||
| result += " " + count + ". " + iterator.next().toString() + "\n"; | ||
| } |
There was a problem hiding this comment.
This part maybe can consider abstracting it into another method (convertListToString or something) because it can be reused (in list() for example) I think?
| * @param storage Storage object which handles storing of data. | ||
| * @param ui Ui that interact with user. | ||
| */ | ||
| public abstract void execute(TaskList taskList, Storage storage, Ui ui); |
src/main/java/duke/Parser.java
Outdated
| * @param input The deadline details given by user. | ||
| * @return A Deadline with the input given. |
There was a problem hiding this comment.
Should there be an empty line between 98 and 99?
There was a problem hiding this comment.
and other javadoc with description and params too 👍
There was a problem hiding this comment.
also maybe the description should start with Returns instead of Return?
src/main/java/duke/Storage.java
Outdated
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } | ||
|
|
There was a problem hiding this comment.
perhaps this space here can let go?
Branch a assertions
Improve code quality.
No description provided.