Skip to content

[Zeng Xunyi] iP#78

Open
XunyiiZ wants to merge 28 commits into
nus-cs2113-AY2122S2:masterfrom
XunyiiZ:master
Open

[Zeng Xunyi] iP#78
XunyiiZ wants to merge 28 commits into
nus-cs2113-AY2122S2:masterfrom
XunyiiZ:master

Conversation

@XunyiiZ

@XunyiiZ XunyiiZ commented Feb 3, 2022

Copy link
Copy Markdown

No description provided.

Comment thread src/main/java/TaskManagement.java Outdated
if(input.equals("list")){
System.out.println("Here are the tasks in your list:");
for(int i =0 ; i<3; i++){
System.out.println((i+1) + ". " + taskList[i].toString());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might be too many nesting

@hjunleon hjunleon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall code quality is ok

Comment thread src/main/java/Greet.java Outdated
input[0] = sc.nextLine();
while(!input[idx].equals("bye")){
if(input[idx].equals("list")){
idx--;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The logic isn't clear. idx -- in this line and idx ++ in line 25

Comment thread src/main/java/Task.java
}

public void markAsDone(){
this.isDone=true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Some have space around the equal sign

@froststein froststein left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Followed some of the coding standard.
Just a couple of spaces to add and remove and it follows the coding standard.

Comment thread src/main/java/Greet.java Outdated
while(!input[idx].equals("bye")){
if(input[idx].equals("list")){
idx--;
for(int i =0; i<=idx ;i++){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For-loop variable declaration could have been more spaced out?

Comment thread src/main/java/TaskManager.java Outdated
System.out.println((i+1) + ". " + taskList[i].toString());
}
}
else if(input.contains("unmark")){

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider else / else if on the same line after the }.
Consider the spaces between ) and {.

Comment thread src/main/java/TaskManager.java Outdated
taskList[2] = new Task("buy bread");

taskList[0].markAsDone();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider reducing the amount of spaces in between lines of code

Comment thread src/main/java/TaskManager.java Outdated
int idx = Integer.parseInt(input.replace("unmark ", ""));
taskList[idx-1].markAsUndone();
System.out.println(taskList[idx-1].toString());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider removing the extra line after every if else block

@limjierui limjierui left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There seems to be 2 main functions, Greet and TaskManager. Maybe can combine both into one to make one of it the main function.
Would be better if you are able to continuously taking in inputs and adding them to tasks instead of having only 3 inputs.
For taking in of commands, maybe it will be better not to replace the words in the commands. Instead, can just split the string and obtain different parts if the commands.
Displayed some OOP concepts such as Classes.

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