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

[Tang-Moyan] iP #542

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

Conversation

Tang-Moyan
Copy link

@Tang-Moyan Tang-Moyan commented Sep 4, 2023

Duke-Rio

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

Duke-Rio helps you store your tasks in a digital list. It's,

  • text-based
  • automaticly saving your changes
  • retrieving your previous list even if you close it by accident.

All you need to do is,

  1. download it from this link
  2. run the ip.jar
  3. add your tasks.
  4. let it manage your tasks for you 😁

And it is FREE!

Features:

  • managing tasks
  • find if you have a certain task
  • Managing deadlines (coming soon)

Here's the main method:

public class Duke {
    public static void main(String[] args) {

            new Duke("list.txt", "data").run();


    }
}

You can modify this line new Duke("list.txt", "data").run(); in the main function to decide where and what file name is used to store your list.

Copy link

@AustinHuang1203 AustinHuang1203 left a comment

Choose a reason for hiding this comment

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

Overall ok and neat code that is readable. Could consider more encapsulation and java doc comments also! But I think it is fine and nice.

import java.io.IOException;
import java.nio.file.Path;
import HelperClass.Task;

public class Duke {

Choose a reason for hiding this comment

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

Hi, not sure which level you are at but you could consider more encapsulation of code 😸

Copy link
Author

Choose a reason for hiding this comment

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

Thank you very much for your review! I am a bit falling behind in catching up with the levels. I will try to get them done sooner.

printOneLine();
}

public static void Exit() {

Choose a reason for hiding this comment

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

double spacing can change to single between public and static

System.out.println("Directory '" + directoryName + "' created.");
} else {
System.err.println("Failed to create directory '" + directoryName + "'.");
return;

Choose a reason for hiding this comment

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

redundant return statement


printOneLine();
switch (userInput) {

Choose a reason for hiding this comment

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

This block of code is kindda big so maybe can encapsulate out

@@ -0,0 +1,3 @@
todo

Choose a reason for hiding this comment

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

Can try to be a bit more comprehensive

public class Task {
private boolean isDone;

private int type;

Choose a reason for hiding this comment

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

Can consider a more clear name beside type for int

Copy link

@SimWPEric SimWPEric left a comment

Choose a reason for hiding this comment

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

Good job on your progress thus far ! A-CheckStyle increment will come in really handy for catching styling errors !

Comment on lines 64 to 147
switch (parser.getCommand()) {

case "bye":
//bye

wantToExit = true;
ui.Speak(ui.Exit());

break;

case "list":
//list
ui.Speak(tasks.displayList());
break;

case "mark":
//mark 1

try {
int i = Integer.parseInt(parser.getTaskName()) - 1;
ui.Speak(tasks.markTask(i));
userListHaveChanges = true;
} catch (NumberFormatException e) {
ui.Speak("need to provide an integer index of task.");
}


break;

case "unmark":
//unmark 1
try {
int i = Integer.parseInt(parser.getTaskName()) - 1;
ui.Speak(tasks.unmarkTask(i));
userListHaveChanges = true;
} catch (NumberFormatException e) {
ui.Speak("need to provide an integer index of task.");
}

break;

case "todo":
//todo read book
ui.Speak(tasks.addTask(new Task(parser.getTaskName(),
1, "Null", "Null", false)));
userListHaveChanges = true;
break;

case "deadline":
//deadline read book /by 2022-01-01
ui.Speak(tasks.addTask(new Task(parser.getTaskName(),
2, "Null", parser.getFirstEnteredTime(), false)));
userListHaveChanges = true;
break;

case "event":
//event read book /from 2022-01-01 /to 2022-01-02
ui.Speak(tasks.addTask(new Task(parser.getTaskName(),
3, parser.getFirstEnteredTime(), parser.getSecondEnteredTime(), false)));
userListHaveChanges = true;
break;

case "delete":
//delete 1
try {
int i = Integer.parseInt(parser.getTaskName()) - 1;
ui.Speak(tasks.deleteTask(i));
userListHaveChanges = true;
} catch (NumberFormatException e) {
ui.Speak("need to provide an integer index of task.");
}

break;

case "find":
ui.Speak(tasks.findTask(parser.getTaskName()));
break;


default:
ui.Speak("OOPS!!! I'm sorry, but 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.

According to Java Coding Standard: The switch statement should have the following form: Note there is no indentation for case clauses.


public class Ui {

private String MyName;

Choose a reason for hiding this comment

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

Try using camelCase for variable names !

Comment on lines 12 to 13
* print message in a specific format
* @param message the original message to be printed

Choose a reason for hiding this comment

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

Can be good to include an empty line between the description and @param !

Comment on lines 7 to 18
public void testGetUserCommand() {
Parser parser = new Parser();
parser.processUserCommand("todo Read Book");
assertEquals("todo", parser.getCommand());
}

@Test
public void testGetTaskName() {
Parser parser = new Parser();
parser.processUserCommand("todo Read Book");
assertEquals("Read Book", parser.getTaskName());
}

Choose a reason for hiding this comment

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

According to CS2103T Website W3.7d code normally use camelCase for method names e.g., testStringConversion but when writing test methods, sometimes another convention is used:unitBeingTested_descriptionOfTestInputs_expectedOutcome.

* return the string representing how the Task object should be stored
* @return string representation of the Task object
*/
public String ForRecordingInTextFile() {

Choose a reason for hiding this comment

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

Try using camelCase for method names !

Tang-Moyan and others added 17 commits September 15, 2023 15:10
getResponse() only return the user input.

Let's update the getResponse() function so it work correctly with duke
and returns the right message.

In this way, duke GUI can do its job correctly.
When we have a lot of tasks in the list, it would be convenient to
have a function that shows the user how many each type of task
there are.

Let's have a new user command, "stats", to invoke the
showTaskStatics() function.

It is a convenient way of showing task information to the user.
We need to check some crucial assumptions at certain time during
execution of the program.

Let's use assert feature to document important assumptions that
should hold at various points in the code.

In this way, users will know more about where went wrong when using
this product.
There are several methods that are too long.

Long methods are harder for other developers to understand.

Shortening them improves readability of the code.

Let's divide long methods into short methods so that other
developers can understand the code more easily.
Task names can be empty based on user input.

Empty task name is should not be allowed becuase it will lead to
bugs.

Limiting the task name to be non-empty prevents bugs.

Let's limit our user to give only non-empty task names, so that no
bugs will occur.
Command functions (todo, deadline, event...) have some
common behaviors (return a message).

Common behaviors should be abstract out for better OOP practice.

Abstracting out common behaviors into a super class and making each
command function a separate class allow more conveninent extensions
of current commands in the future.

Let's have a super class Command and define all commands as child
classes of the Command class.

Using inheritance is preferable over composition in this situation
because the common behaviors are not composable.
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.

4 participants