[Ming Gao Rickie Li] iP #469

MGRL2201 · 2021-08-26T10:26:43Z

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,

EXTREMELY FAST to use
easy to learn
text-based

All you need to do is,

download it from here
double-click it.
add your tasks with todo, event or deadline commands
let it manage your tasks for you 😉

Best of all, Duke is FREE

Features:

Managing Tasks
Managing Deadlines (coming soon)
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) {
    new Duke("Data\\taskList.txt").run();
}

Justinhoejj

Hey, code looks acceptable to me. Gave some comments on how you could further abstract some methods, hope that will help in improving the code quality. Do note the naming of test methods as well. Otherwise good job on the project! 👍

Justinhoejj · 2021-08-31T01:42:31Z

src/main/java/duke/Deadline.java

+public class Deadline extends Task {
+
+ protected String by;
+ protected LocalDate date;


If these variables are not used in other parts of the code base consider changing the accessor to private.

Justinhoejj · 2021-08-31T01:44:43Z

src/main/java/duke/Deadline.java

+ protected String by;
+ protected LocalDate date;


Consider declaring variables as final if the attributes are not expected to change.

Justinhoejj · 2021-08-31T01:46:43Z

src/main/java/duke/Duke.java

+ private Storage storage;
+ private Ui ui;
+ private String filePath;


Consider declaring as final.

Justinhoejj · 2021-08-31T01:51:17Z

src/main/java/duke/Event.java

+ protected String at;
+ protected LocalDate date;


Consider changing to private final.

Justinhoejj · 2021-08-31T01:54:52Z

src/main/java/duke/Parser.java

+ if (input.replaceAll("\\s","").toLowerCase().equals("bye")) {
+ return "bye";
+ } else if (inputLower.replaceAll("\\s", "").equals("list")) {
+ return "list";
+ } else if (inputLower.length() > 3 && inputLower.substring(0, 4).equals("done")) {
+ return "done";
+ } else if (inputLower.length() > 5 && inputLower.substring(0, 6).equals("delete")) {
+ return "delete";
+ } else if (inputLower.length() >= 4 && inputLower.substring(0, 4).equals("todo")) {
+ return "todo";
+ } else if (inputLower.length() >= 5 && inputLower.substring(0, 5).equals("event")) {
+ return "event";
+ } else if (inputLower.length() >= 8 && inputLower.substring(0, 8).equals("deadline")) {
+ return "deadline";
+ } else if (inputLower.length() >=4 && inputLower.substring(0, 4).equals("find")) {
+ return "find";
+ } else {
+ return "InvalidCommand";
+ }


Consider using case switch. Additionally, consider implementing a general remove first word method that extracts the string before the first space.

Justinhoejj · 2021-08-31T01:57:23Z

src/main/java/duke/Storage.java

+ fileReader.close();
+ }
+ } catch (IOException e) {
+ System.out.println("An error occurred.");


Consider specifying the type of error. eg. error loading tasks.

Justinhoejj · 2021-08-31T02:02:38Z

src/main/java/duke/Ui.java

+ System.out.println(separator);
+ System.out.println(" You have no tasks in your list!");


A UI component that can be abstracted to a generic method to format responses. Since these lines of code are used extensively in the run method, abstracting the method will reduce code re-write .

Justinhoejj · 2021-08-31T02:04:25Z

src/main/java/duke/Ui.java

+ System.out.println(" ____________________________________________________________");
+ System.out.println(" Got it. I've added this task:");
+ System.out.println(" " + t.toString());
+ System.out.println(String.format(" Now you have %d tasks in the list.", list.size()));


These lines of codes are used more than twice. Consider abstracting to a method to enhance ease of readability and maintainability.

Justinhoejj · 2021-08-31T02:09:07Z

src/main/java/duke/Ui.java

+ Task t = list.get(i);
+ if (list.get(i).description.toLowerCase().contains(query)) {
+ tasksContainingQuery.add(t);
+ }
+ }
+ if (tasksContainingQuery.size() == 0) {


Logic here involves the task list. It may be more appropriate to extract into a method in a task list class. Ideally UI takes string from user to pass to pass to parser and takes string from duke and prints, since the UI is an interface between duke and user. All other logic can be defined in other classes.

Justinhoejj · 2021-08-31T02:10:05Z

src/test/java/DukeTest.java

+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class DukeTest {


Consider following the test method naming convention featureUnderTest_testScenario_expectedBehavior().

HolmesJJ

I like your clear code and it is esay to understand. 👍 Perhaps more comments for the class and method will be much easier to understand. 😄

HolmesJJ · 2021-09-01T09:36:20Z

src/main/java/duke/Deadline.java

+package duke;
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+public class Deadline extends Task {


Perhaps need a descriptive header comments for the Parser class? 🤔

HolmesJJ · 2021-09-01T09:37:59Z

src/main/java/duke/Deadline.java

+ protected String by;
+ protected LocalDate date;
+
+ public Deadline(String description, String by) {


Perhaps need a descriptive header comments for the Deadline method? 🤔
And I noticed the same issue in several other places below.

HolmesJJ · 2021-09-01T09:38:31Z

src/main/java/duke/Duke.java

+
+import java.util.Scanner;
+
+public class Duke {


Perhaps need a descriptive header comments for the Duke class? 🤔

HolmesJJ · 2021-09-01T09:39:04Z

src/main/java/duke/Duke.java

+ private Ui ui;
+ private String filePath;
+
+ public Duke(String filePath) {


Perhaps need a descriptive header comments for the Duke method? 🤔
And I noticed the same issue in several other places below.

HolmesJJ · 2021-09-01T09:39:34Z

src/main/java/duke/Duke.java

+ }
+
+ public static void main(String[] args) {
+ new Duke("C:\\Users\\Rickie\\Documents\\University\\Year 2\\Semester 1\\CS2103T\\ip\\Data\\taskList.txt").run();


Should this be wrapped to next line? 🤔

HolmesJJ · 2021-09-01T09:47:28Z

src/test/java/DukeTest.java

+ @Test
+ public void uiWelcomeTest() {
+ String separator = " ____________________________________________________________";
+ String message = separator + "\n" + " Hello! I'm Duke" + "\n" + " What can I do for you?" + "\n" + separator;


Perhaps need an empty line between description and parameter section? 🤔

HolmesJJ · 2021-09-01T09:47:59Z

src/test/java/DukeTest.java

+ Duke duke = new Duke("C:\\Users\\Rickie\\Documents\\University\\Year 2\\Semester 1\\CS2103T\\ip\\Data\\taskList.txt");
+
+ @Test
+ public void parserTest1() {


Perhaps need a descriptive header comments for the parserTest1 method? 🤔
And I noticed the same issue in several other places below.

HolmesJJ · 2021-09-01T09:48:22Z

src/main/java/duke/Ui.java

+ }
+
+ /**
+ * Returns updated user task list and prints feedback to the user based on their input


Perhaps need a full stop at the end? 🤔

HolmesJJ · 2021-09-01T09:48:59Z

src/main/java/duke/Todo.java

+package duke;
+public class Todo extends Task {
+
+ public Todo(String description) {


Perhaps need a descriptive header comments for the Todo method? 🤔
And I noticed the same issue in several other places below.

HolmesJJ · 2021-09-01T09:49:11Z

src/main/java/duke/Task.java

+ protected String description;
+ protected boolean isDone;
+
+ public Task(String description) {


Perhaps need a descriptive header comments for the Task method? 🤔
And I noticed the same issue in several other places below.

Chesterwongz

LGTM!

Chesterwongz · 2021-09-02T14:31:32Z

src/test/java/DukeTest.java

+
+public class DukeTest {
+
+ Duke duke = new Duke("C:\\Users\\Rickie\\Documents\\University\\Year 2\\Semester 1\\CS2103T\\week 2\\ip\\Data\\taskList.txt");


Consider using relative path instead of absolute path, incase project structure changes in the future

Chesterwongz · 2021-09-02T14:37:18Z

src/main/java/duke/Todo.java

@@ -0,0 +1,12 @@
+package duke;


Minor style issue here, but there should always be a new line after package statement

Chesterwongz · 2021-09-02T14:39:26Z

src/main/java/duke/Ui.java

+ System.out.println(String.format(" %d. %s", (i + 1), item.toString()));
+ }
+ }
+ } else if (parsedInput.equals("done")) {


perhaps the throwing of exceptions should be done in the Parser rather than Ui handler, since the parser handles valid and invalid input

There is no code in place to indicate possible bugs. Assertion statements have been added to Duke class to ensure user input and program output are not null. The impact of assertions on performance is low and provides a safety net to ensure the user input and program output are as expected.

Branch A-Assertions

The code needs to be more readable and abstract. Further abstracting code has made the code easier to read and follow along. Abstracting shortens method length and good naming of abstracted methods allow for more readable code.

Code quality is not great.

added update function

ramapriyan912001

Great job overall!
Just some tweaks needed!

ramapriyan912001 · 2021-09-16T09:31:41Z

src/main/java/duke/Event.java

+import java.time.format.DateTimeFormatter;
+public class Event extends Task {
+
+ protected String at;


"at" is perhaps not descriptive enough

ramapriyan912001 · 2021-09-16T09:32:08Z

src/main/java/duke/Event.java

+ if (this.date != null) {
+ return "[E]" + super.toString() + " (at: "
+ + this.date.format(DateTimeFormatter.ofPattern("MMM d yyyy")) + ")";
+ } else {
+ return "[E]" + super.toString() + " (at: " + this.at + ")";


Some use of magic literals here.

ramapriyan912001 · 2021-09-16T09:32:56Z

src/main/java/duke/Parser.java

+ public static String parse(String input) {
+ String inputLower = input.toLowerCase();
+ if (input.replaceAll("\\s", "").toLowerCase().equals("bye")) {
+ return "bye";
+ } else if (inputLower.replaceAll("\\s", "").equals("list")) {
+ return "list";
+ } else if (inputLower.length() > 3 && inputLower.substring(0, 4).equals("done")) {
+ return "done";
+ } else if (inputLower.length() > 5 && inputLower.substring(0, 6).equals("delete")) {
+ return "delete";
+ } else if (inputLower.length() >= 4 && inputLower.substring(0, 4).equals("todo")) {
+ return "todo";
+ } else if (inputLower.length() >= 5 && inputLower.substring(0, 5).equals("event")) {
+ return "event";
+ } else if (inputLower.length() >= 8 && inputLower.substring(0, 8).equals("deadline")) {
+ return "deadline";
+ } else if (inputLower.length() >= 4 && inputLower.substring(0, 4).equals("find")) {
+ return "find";
+ } else if (inputLower.length() >= 6 && inputLower.substring(0, 6).equals("update")) {
+ return "update";
+ } else {
+ return "InvalidCommand";
+ }
+ }


Well done, very neat, and handles commands of different cases!

ramapriyan912001 · 2021-09-16T09:33:24Z

src/main/java/duke/Storage.java

+ } else {
+ Event e = new Event(splitString[2], splitString[3]);
+ if (splitString[1].equals("1")) {
+ e.markAsDone();


Could perhaps be SLAP-ed harder?

ramapriyan912001 · 2021-09-16T09:33:59Z

src/main/java/duke/Ui.java

+ * @param input user input
+ * @return updated task list
+ */
+ public String run(Storage storage, String input) throws DukeException {


This method could be SLAP-ed harder perhaps?

ramapriyan912001 · 2021-09-16T09:34:16Z

src/main/java/duke/Ui.java

+ }
+ }
+
+ private String getDeleteTaskString(ArrayList<Task> list, String input, Storage storage) throws DukeException {


Well modularised! Great job!

ramapriyan912001 · 2021-09-16T09:34:51Z

text-ui-test/EXPECTED.TXT

+ 2. [T][ ] read book
+ 3. [D][X] return book (by: Sunday)
+ 4. [E][ ] project meeting (at: Monday 2-4pm)
+ ____________________________________________________________


The line separation definitely helps. Good job!

updated user guide

MGRL2201 added 8 commits August 18, 2021 21:28

Level 1. Greet, Echo, Exit

016ad8d

Level 2. Add, List

d81e3b0

Level 3. Mark as Done

87638eb

Level 4. ToDos, Events, Deadlines

7f141e7

A-TextUiTesting: Automated Text UI Testing

ad65ad1

Level 5. Handle Errors

2a2efbd

Level 6. Delete

974d93a

A-Enums: Use Enums

b4b212f

MGRL2201 changed the title ~~[Ming Gao Rickie Li]~~ [Ming Gao Rickie Li] ip Aug 26, 2021

MGRL2201 changed the title ~~[Ming Gao Rickie Li] ip~~ [Ming Gao Rickie Li] iP Aug 26, 2021

MGRL2201 added 12 commits August 26, 2021 19:49

Level 7. Save

8a2b75e

Level 7. Save

5927253

Level 7. Save

344c259

Level 8. Dates and Times

eddbc35

Merge branch 'branch-level-8'

e376a76

Use More OOP

f334294

Organize into Packages

1e24a6c

Add JUnit Tests

04d6408

Create a JAR File

0e270b5

JavaDoc

bd59051

Follow the Coding Standard

8ad2179

Level 9. Find

3369de3

Justinhoejj reviewed Aug 31, 2021

View reviewed changes

HolmesJJ reviewed Sep 1, 2021

View reviewed changes

MGRL2201 added 3 commits September 2, 2021 17:33

A-Gradle

c12c5a7

A-Gradle

2e99d62

A-Checkstyle

22c64b4

Chesterwongz reviewed Sep 2, 2021

View reviewed changes

MGRL2201 added 2 commits September 3, 2021 01:05

Level-10

ed5c92c

MGRL2201 and others added 5 commits September 9, 2021 14:43

Merge pull request #1 from MGRL2201/branch-A-Assertions

4155640

Branch A-Assertions

Code quality is not great.

3a9ebac

The code needs to be more readable and abstract. Further abstracting code has made the code easier to read and follow along. Abstracting shortens method length and good naming of abstracted methods allow for more readable code.

Merge pull request #2 from MGRL2201/branch-A-CodeQuality

3b2f505

Code quality is not great.

added update function

472fac4

Merge pull request #3 from MGRL2201/BCD-Extension

07688eb

added update function

ramapriyan912001 reviewed Sep 16, 2021

View reviewed changes

MGRL2201 and others added 8 commits September 18, 2021 00:55

polished code

caf4dfa

A-BetterGui

1a9c8fe

minor updates to commands

d75e934

added assert messages

91a4400

Set theme jekyll-theme-tactile

341b156

Update README.md

61841c2

updated user guide

A-Release

cafd45e

Merge branch 'master' of https://github.com/MGRL2201/ip

5716959

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ming Gao Rickie Li] iP #469

[Ming Gao Rickie Li] iP #469

MGRL2201 commented Aug 26, 2021 •

edited

Loading

Justinhoejj left a comment

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

Justinhoejj Aug 31, 2021

HolmesJJ left a comment

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

HolmesJJ Sep 1, 2021

Chesterwongz left a comment

Chesterwongz Sep 2, 2021

Chesterwongz Sep 2, 2021

Chesterwongz Sep 2, 2021

ramapriyan912001 left a comment

ramapriyan912001 Sep 16, 2021

ramapriyan912001 Sep 16, 2021

ramapriyan912001 Sep 16, 2021

ramapriyan912001 Sep 16, 2021

ramapriyan912001 Sep 16, 2021

ramapriyan912001 Sep 16, 2021

ramapriyan912001 Sep 16, 2021

		System.out.println(separator);
		System.out.println(" You have no tasks in your list!");


		import static org.junit.jupiter.api.Assertions.assertEquals;

		public class DukeTest {


		public class DukeTest {

		Duke duke = new Duke("C:\\Users\\Rickie\\Documents\\University\\Year 2\\Semester 1\\CS2103T\\week 2\\ip\\Data\\taskList.txt");

[Ming Gao Rickie Li] iP #469

Are you sure you want to change the base?

[Ming Gao Rickie Li] iP #469

Conversation

MGRL2201 commented Aug 26, 2021 • edited Loading

Duke

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

All you need to do is,

Features:

Justinhoejj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HolmesJJ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chesterwongz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramapriyan912001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MGRL2201 commented Aug 26, 2021 •

edited

Loading