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

[Tan Zheng Fu Justin] iP #62

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

Conversation

JustinnT
Copy link

No description provided.

Copy link

@sliu107 sliu107 left a comment

Choose a reason for hiding this comment

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

Reviewed with @Zhilin-Huang

import java.util.List;

public class Storage {
private List<Task> myTasks = new ArrayList<>();
Copy link

Choose a reason for hiding this comment

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

Is it enough to just import ArrayList class? Since I think you can directly define the type of myTasks as ArrayList.

return myTasks.get(index - 1);
}
catch(Exception e) {
return null;
Copy link

Choose a reason for hiding this comment

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

Maybe you can use Printer to give some error messages if any exception has been caught.

String userInput;

userInput = in.nextLine();
Task task = new Task(userInput);
Copy link

Choose a reason for hiding this comment

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

Do you agree that the description of a task is actually the part after the first word of the user input? ( If according to the user input, we need create a task)

public static void echoUntilBye() {
boolean isBye = false;
while (!isBye) {
Task task = readFromUser();
Copy link

Choose a reason for hiding this comment

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

I think we can create a new Task only after we get the command which requires to do so.Since if the command is "list" or "bye", then there is no need to so.

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