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

[Francis Tan] iP #522

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

Conversation

Francis-Tan
Copy link

@Francis-Tan Francis-Tan commented Sep 2, 2022

Duke

“Your mind is for having ideas, not holding them.” – David Allen (source)

Duke frees your mind of having to remember things you need to do. It's

  • Text-based
  • Easy to learn
  • Fast SUPER FAST to use

All you need to do is

  1. Download it from here
  2. Save it into a folder
  3. Add your tasks
  4. Let it manage your tasks for you 😉

And it is FREE!

Features:

  • Organize tasks into 3 types: Todos, Deadlines, Events
  • Mark and unmark any task as done
  • Type in words and find matching tasks
  • Specify date and optionally time for Deadlines and Events
  • Reminders (coming soon)

If you're a Java programmer, you can use it to practice Java too. Here's the main method:

public static void main(String[] args) {
    Application.launch(Main.class, args);
}

…ead of reading the input itself, now read words from the input split by " ", to make code more readable
…n optional (h:mm)am/pm, and the description shows the month in english
Copy link

@rui-han-crh rui-han-crh left a comment

Choose a reason for hiding this comment

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

Very nice! Code is clean and readable, good amount of test cases to cover many bases.

package duke.services;

/** Handles display */
public class UI {

Choose a reason for hiding this comment

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

Perhaps the name of this class would be better written as Ui, to conform to the coding standard?

Comment on lines 27 to 49
if (words.length == 1 && words[0].equals("list")) {
TaskList.listTasks(); //could put words.length == 1 cases all here
} else if (words.length == 1 && words[0].equals("SAVE")) {
Storage.wipeDataOnExit(false);
} else if (words.length == 1 && words[0].equals("WIPE")) {
Storage.wipeDataOnExit(true);
} else if (words[0].equals("todo")) {
TaskList.addTodo(words);
} else if (words[0].equals("deadline")) {
TaskList.addDeadline(words);
} else if (words[0].equals("event")) {
TaskList.addEvent(words);
} else if (words[0].equals("mark")) {
TaskList.markTaskAsDone(words);
} else if (words[0].equals("unmark")) {
TaskList.markTaskAsNotDone(words);
} else if (words[0].equals("delete")) {
TaskList.deleteTask(words);
} else if (words[0].equals("find")) {
TaskList.findTasksContainingKeyword(words);
} else {
UI.sayLines(new String[]{"I'm sorry, I don't know what that means"});
}

Choose a reason for hiding this comment

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

Perhaps a switch statement would be more conventional for readibility here?

*/
public static void handleUserInputs() {
Scanner inputScanner = new Scanner(System.in);
String[] words = Arrays.stream(inputScanner.nextLine().strip().split(" ")).toArray(String[]::new);

Choose a reason for hiding this comment

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

Nice application of declarative input consumption and splitting here! :)

assertEquals(Parser.getDescription(deadline1, "/by"), "finish essay");
assertEquals(Parser.getTiming(deadline1, "/by"), "tmr night or ask 4 extension");
//does it throw exception for missing all other arguments?
assertThrows(IllegalArgumentException.class, () ->

Choose a reason for hiding this comment

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

I like how you also decided to test the exceptions here!

But this test case may be testing too many things at the same time, perhaps split these tests into more subgroups? Like
testGetDescription_event, testGetDescription_deadline, testGetDescriptionAndDate_throwsIllegalArgumentException, etc

Refactored code by making Duke, rather than the parser, decide how to
respond to inputs, and merged UI with Duke, since UI was in charge of
too little functionality that Duke could also handle reasonably.
Broke down a very long test case into smaller test cases.
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