iP review comments dashboard

it should have been

if(taskCommandArr[1].contains("/by")) {

A space after the last ")"

Should this blank line be removed in order to collect all the import statements together?

I think this blank line can be removed to make the code more elegant

Should this be split out into methods instead? It might make the code more readable?

For eg maybe a function named userEnteredDeadline or userEnteredEvent

I think this blank line can be removed to make the code more elegant

Should this be refactored out to reduce the arrow head structure?

I think this blank line can be removed to make the code more elegant

I feel the comment was sufficient in showing what u2713 and u2718 meant.

Slightly unreadable as it is hard to tell what you mean by this. Perhaps refactoring out into methods would be a good idea.

A few common lines of code can be refactored out to reduce duplication. The lines are :

arrayOfTasks.add(newTask);

setNumberOfTasks(getNumberOfTasks()+1);

printAddTaskMessage(newTask);

Perhaps they could go in a method and call that method instead?

Should taskToBeDone be taskNumberToBeDone

Variables should be initialized where they are declared to ensure that it is valid at any time, consider moving it to line 16 so it's "String text = in.nextLine();".

Variables should be initialized where they are declared to ensure that it is valid at any time so consider moving them to when you are gonna assign them values.

Variables should be initialized where they are declared to ensure that it is valid at any time so consider moving it to line 48, so it's "String todoDescription = text.substring(5);".

There should be whitespace within a statement, so consider "public static void addTodoTask(String text) {".

There should be whitespace within a statement, so consider "public static void addDeadlineTask(String text) {".

There should be whitespace within a statement, so consider "public static void addEventTask(String text) {".

There should be whitespace within a statement, so consider "public static void printTask(Task task) {".

There should be whitespace within a statement, so consider "public class Todo extends Task {", similarly for line 6 and 10.

There should be whitespace within a statement, so consider "public void taskDone() {", similarly for line 19.

There should be whitespace within a statement, so consider "public class Event extends Task {", similarly for line 6, 9 and 13.

There should be whitespace within a statement, so consider "this.byDate = byDate", similarly for line 10 and 14.

Might wanna consider using a line break for this as it's pretty lengthy.

Try using a constant variable for "100" so it does not appear to be a magic number.

Consider using a case-insensitive comparison method as that can simplify the expression, a boolean variable to store the validity checks result would also be good too to simplify it further.

Consider using a constant variable to represent "100" so it isn't a magic number.

Try to avoid complication expressions such as this, maybe use a task variable to store the "list[taskNumInt - 1]" and also do use whitespaces to separate it out, it can improve the readability.

Consider using a constant variable named "TICK_SYMBOL" for "\u2713" otherwise it is a magic value in the code (same for "\u2718").

Try to avoid complication expressions like "((NUM_OF_TASKS==1?"":"s"))", maybe use a String variable to store the result first, then use that variable for printing.

Consider using a better variable name like 'task' or 'newTask' instead of 't', similarly in line 102 and 117.

Try to avoid complication expressions such as this, consider using the toString() function in the Task class to simplify the expression.

Very similar chunks of code (line 97 to 120), consider generalizing with a createTask method (also with todo if possible)?

You might want to minus 1 at line 146 ("int taskNumInt = Integer.parseInt(taskNumString);") so you don't have to repeat minus 1 every time you need to access it, that would also help to simplify your code further.

You may wanna set the access modifier to private to limit the scope of these methods if they are not gonna be used outside of the Duke class. Similarly for the other methods in Duke.

The curly braces for the if statement is missing, it is needed even if it's a one-liner (K&R bracket style).

Boolean variables should sound like a boolean, for example. isQuittingLoop.

Bug here. If command does not contain any spaces, it falls through all the branches and commandDetails is still null.

Is an else if missing?

Good spacing! Very pleasant to read 👍

The spacing in this file is inconsistent with other files...

The todo, deadline and event commands are indented on a different level from the Done command. Try to put them on the same indentation level, or bring the handling of these commands to another method and call that method on the same indentation level as other commands.

This class follows coding standard well, good job!

Consider adding in comments for the branched portion

Consider declaring this variable before you declare greeting, and make use of it.

Consider moving "deadline" to the same indentation level as your "todo".

Consider creating a boolean value to replace this long condition.

Spacing should be consistent around operators.

Method name should be a verb. For example, printStartMsg() or showStartMsg().

Unreadable variable name detected. What is dsc? The string name should be more readable.

Method name should be a verb. For example, printByeMsg() or showByeMsg().

Spacing should be either on both sides of the equal sign or neither. Having a space only on one side is inconsistent.

Too many empty lines between statements.

Name of methods should be verb followed by action. For example, printLineSeparator() instead of lineSeparator()

Consider using "/.../" for multiple-line comments

Variable name should be a noun to differentiate from methods. For example, userText instead of storeUserText which sounds like a method.

Boolean names should sound like a boolean. For example, "isExitting", "isRunning", etc, instead of "doExit" which sounds like a method.

Method name should be a verb followed by a noun, for example, getLineSeparator() or printLineSeparator()

Everything inside a try...catch block should be indented. They cannot be on the same level of indentation as "try" itself.

Cases without a "break" should be followed by a comment saying that this is intentional.

Could this be named a bit more descriptively? As it stands I'm not entirely sure what this variable is for.

Ditto for here, perhaps the name could be more descriptive about the function/use of the variable rather than the content?

Same as above.

Might it be a good idea to extract this out as a constant so you call that, instead of repeating this line of code over and over?

Would a for loop be appropriate here?

Perhaps this variable could be named more clearly so it's more obvious what it's for?

Not sure if this is a violation of any standards per se, but would this method be better as a non-static method that modifies the specific instance? It might clean up some of the code where you call it too. Is there any reason this is static?

I like this, an ingenius solution in my opinion.

Not sure if this is a standards violation, but OOP best practice as far as I am aware is to grant as little access as necessary. Should this perhaps be private instead of protected?

Perhaps this would be better as a constant (final, with appropriate naming style)?

Same here, perhaps it would be appropriate to make this a constant.

Not sure if this violates any standards in the "code quality" guidelines, but OOP best practice as far as I am aware is to only allow as much access as strictly necessary. Perhaps this should be private instead of protected?

Nice use of SLAP 😃

I'm not sure of the purpose of this line. Perhaps clearer variable naming or a comment would be appropriate here?

Not sure if this blank line is necessary? Is it for readability? I think it would be fine without, personally.

Could this be simplified (perhaps with some SLAP) to avoid the dreaded "arrowhead code"?

Perhaps this (and the conditions above) would be a good section of code to SLAP?

Could this be made a constant to avoid code duplication?

Could this be private?

Consider assigning input.split("todo")[1].trim() to a variable (for example description) that represents the purpose of this argument?

Consider assigning input.split("event")[1].trim() to a variable (for example eventDetails) that represents the purpose of this argument?

Consider assigning input.split("event")[1].trim() to a variable (for example deadlineDetails) that represents the purpose of this argument?

Need a space before time.

Need a space before tasks.

Can drop the this.

Drop the this.

Drop the this. Method block should be in a new line.

Drop the super.

Add a space before {.

Remove the super..

Unused import.

Remove the super..

Drop the this..

Perhaps, you should name a class with PascalCase manner as stated in Coding Standard(for example: ToDo)

Pay attention to the naming of variable so todo should be toDo

Hey! just an advise: you can use a constant for the array like MAX_LIST_SIZE

perhaps, the naming should be addedToDo

Hey I like how you created a method for the output format!

Hello! Perhaps, you can add comment on this one for signifying a conversion (for example: // convert taskIndexString into an integer)

Hello! you may pay attention to above. the ); is better in line 55 or you can use what you've done before in printing message(Egyptian style)

Hello! you may pay attention to above. the ); is better in line 33 or you can use what you've done before in printing message(Egyptian style)

Hello again! perhaps, you can make this in line 115 such that it is not redundant like "String userInput = input.nextLine(); and also can comment it as //scan user input

Hello! perhaps, you should pay attention to the naming (camelCase style) like commandTxt

Again, the naming should be in camelCase like if just one word, it would be "message" for such

Hey! you can reformat each conditional to do specific method like if (Message[0].equals("done")) { done();}

perhaps, you should name the variables in camelCase

perhaps, after +, there should be spacing to increase readability

you can make it with just one System.out.println("...\n"). perhaps, use "\n" after each sentence

Variable line might not be very descriptive?

Perhaps there is an extra newline here?

Could there be an unnecessary newline here?

Maybe you want to cleanup comments like these?

Variable in is not might not be very descriptive

Please add a space between else if and statement.

Consider adding classes like Task into a separate file

Please consider adding comments to your methods to explain them.

Consider having space between conditions?

Variable names like position could be ambiguous

Variable in might be ambiguous

You might want to consider reducing code reuse. I noticed how substring is being used multiple times for event/deadline etc. You could implement a method for this.

You could consider making a method that returns both taskToFinish.getStatusIcon() + " " + taskToFinish.description

Consider making constant for magic numbers/literals.

I like how you used constant for line, could be used for others too.

Avoid magic numbers! extract it as a constant like MAX_TASKS to be clearer!

This if-else block is a bit long. Refactoring it into extracted methods would make the code clearer!

Printed messages should be extracted as constant strings for easy editing (and localisation)

Best to use \u???? instead of the symbol so it can be saved properly when testing

This if else block is a bit long. You could extract methods for the individual commands to improve readability 😄

The printed logo, and the greeting message could be extracted and printed using a single method eg printIntroMsgs() so it appears as 1 line in main

For the classes you could add setters and getters as part of encapsulation?

printed messages could be extracted as constants to the top of the code for easy editing (and localisation if needed)

Magic number 100 could be extracted as MAX_TASKS for clarity

Maybe the function names can be a bit more descriptive like for this one what prints the full task list it could be printTaskList or something?

you could create setters and getter functions as part of encapsulation? (i think)

This may count as excessive use of the this word?

https://se-education.org/guides/conventions/java/index.html#variables

https://github.com/nus-cs2113-AY2021S1/forum/issues/27

Could break up this long function into smaller parts for better readability

Maybe you can try refactoring your statements as a constant if you need to print a few lines at the same time.

(eg GREETING for this block) and only need 1 line of code to print it (ie System.out.println(GREETING)😉

You could state the function description for the listTasks as a comment if you want to be super clear.

Good code quality!

Consider using switch statements for user-readability.

Consider using constants instead to reduce number of magic literals.

Consider encapsulating the handling of Tasks under a new class called TaskManager to handle add, get, sets, mark done. This allows for better abstraction of data and functions. This will allow Duke to handle more of the CLI matters such as printing and handling user inputs.

So every task has a reference to the taskList? Then it is irrelevant to store a Task[] in Duke. Instead, static setters and getters could be used.

Good! Follows all basic coding standards laid out in the syllabus. In the case of actionType, you can consider using constants to reduce magic literals.

Unnecessary to use "this" in reference to the properties isDone and category when there are no similarly named arguments passed into the method.

"this" keyword should be used when the arguments passed in into the constructor is similarly named to the properties of the object.

Similar issue observed in Event as well as Deadline

Similar to all the subclasses of Task

"this" keyword should be utilised when the arguments are named similarly to the properties

Consider refactoring to use more constants and reduce the number of magic literals.

Consider abstracting Task, Todo, Deadline and Event to separate files rather than nesting them as a static class.

This allows for better encapsulation of functionality and data to the relevant classes, improving user readability.

Consider creating 2 functions to improve abstraction here:

"Add the new task into the "items" array and to +1 to itemCount"
"Obtain due/at based on indexDivider and obtain the input".

Should the case statements be on the same indentation with the switch statement?

I think this could be changed to a constant variable by using 'final'?

example:

public static final INDENT_SPACE = " ";

Should this 'else' be on the same line as the closing brace of the 'if' statement?

The Task name newObj is not very obvious later on, perhaps it could benefit code readability if it was changed to newTask?

To add on to what Zhi-You have pointed out, perhaps this line could be refactored out as a method? This would help the code adhere closer to SLAP

I like how this was method was formatted, very nice!

I feel that your code is well-written and easy to understand!

One comment I have though is whether the magic string literal and magic number such as "/by" and "4" could be extracted out as a constant variable to improve code quality?

Should the variable 'i' be renamed to something more descriptive?

example: totalTaskAdded

I feel the code's readability can be benefited more with an '@Override' comment for overridden methods, even though it is only optional.

Perhaps these two lines could be refactored out as a method? It could help ensure that processCommands() adhere to SLAP more closely.

e.g. getUserInput(), splitCommand().

Perhaps this code block could be extracted into a new method? It will help improve code readability by following SLAP. It could be similar to your goodbye() method.

Should this method be removed? It does not seem to be in use.

Perhaps this list Task could be removed? It does not seem to be in use.

I think the variable name 'sum' could be replaced with something more descriptive, as it will help code readability!

example: totalTasks

'deadline' may be confusing, maybe could change to deadlineDate?

Maybe you could change executeCommand to accept no parameters and start accepting the userInput inside the top of the while loop of the method for clarity?

Not sure but java.util.Arrays may be unused, you should remove if it is unused

Good SLAP in this method!

Variable name like echo_msg should be in camelCase and not have an underscore

I think the brackets for case are unnecessary but it's up to you if you want to keep them

variable should be camelCase

Nice job with indentation for wrapped lines to make sure line length isnt too long

Good job with the case formatting when using switch

Correct format for try-catch statement

I think you should add break; for default

Good job on the explicit imported class

I think you should have a spacebar after the brackets in printTask() to keep consistency, there are a few methods that does not have this too like your addNewTodo, addNewDeadline etc.

I think space bar is needed after the if and between the bracket for consistency throughout, some other instances of these in your code

I would suggest using camel case for variables. How about timeFrame?

I do not really understand this number 2. Maybe you can convert this into a named constant for better readability?

Since this is a collection of tasks, may I suggest using a plural form of the variable such as tasks?

Plural form should be used when naming collection of arrays. Maybe this can be named as lists instead.

Maybe you need to re look at the correct form of if-else statements similar to the coding standards.

Maybe have a spacing between the parameters for readability.

Maybe the if - else statement can follow the Java convention.

Maybe have a spacing between the round brackets and curly brackets.

Maybe consider method to have a verb name such as printGoodbyeMessage.

I think there should be a space between the round and curly brackets

Maybe you can convert the number 100 into a named constant for better understanding. I suggest naming it as MAX_TASK_SIZE

Maybe consider changing the name of the method to be more of a verb, such as describeTask.

However if the purpose if to print out information about the object, maybe using a toString method would be useful

You can consider replacing the string literals with enums for more readability.

I like how you used the System.lineSeparator() for newline so that it would not be platform dependent.

Is there a reason to use if else statement? I personally think that a switch statement would slightly improve the code's efficiency when dealing with multiple choices. It would also make it easier to read and edit in the future.

the addTask method's if else statement could perhaps merge with this if else statement?

Good job on your indentation for switch statement.

A better naming convention in plural form for this variable would help in code readability.

Is there a reason to put this possible method as a whole class on its own?

No problem! I am actually referring to the String array “split” as I thought since it is an array, it may be better to be named “parts” “words” or “commands”, generally in plural form for more meaning.

Good indentation for the the switch statement.

I like how all the constants are named and used in the code. Really improved the code's readability.

Just curious if the name LS would be confusing as NEWLINE seems to be more intuitive.

Is there a reason to include this.toString() in the constructor?

Since this enum is limited to the Task class, it may be better to declare it in the class to limit its scope.

Also, since enum is a group of constants, it is better to use all uppercase.

Need a 'space' before the curly bracket

Can omit this at this.isDone to make the code neater

Space before the curly bracket

Space before curly bracket

Good work!

Well done!

The line should be assigned as a constant for convenience

Can diversify the commands adapting the different forms of the same commands(ie cases with capitalized letters

need additional space after the "if"s

the horizontal bar should be called as a constant for convenience

Good work!

Should use constants for assigning to these => enhance readability and facilitate coding

You may want to list out the Scanner only instead of all the java util?

Shouldn't the naming be camelCase?

Is there any other better naming instead of 'in'?

This naming is clear, good.

Shouldn't these be all capital letters? (TODO, EVENT, ... )

Is "in" a good naming?

Should there be a space after Task ?

I noticed the same issue in several other places too.

You have separated a class to handle the commands. I like it.

This is good.

I like how clean this bit of code is.

The naming are good and easy to understand.

Should there a spacing after the command.isEmpty()?

This is good.

Good readable code, no deep nesting/arrowhead style

In general the expressions are relatively simple. Also no magic numbers

The code is also largely KISS compatible, easy for readers

SLAP is also implemented appropriately

Good SLAP implementation to de-clutter the main function.

Constants should be in UPPER_CASE format

Constant should be UPPER_CASE

UPPER_CASE

correct indentation for comment

correct array format x[] y

correct mention of lack of break statement in comment

correct indentation for for case statements

correct naming of functions

correct boolean naming

Nice! You add conditions and behaviors where there is no task done

Good quality of code to make the line that are printed over and over into method so that it improves readability of codes.

Great Job! Your code adhere to the coding standards.

There should be spacing between } and else if for this loop (Coding standards)

Shouldn't this task class be its own class file? But overall, the coding is neat and easy to understand. Good job .

Switch Case : For each case statement no indent is need (Coding Standard)

switch (condition) {

case ABC:

statements;

// Fallthrough

Single statement conditionals should still be wrapped by curly brackets.

Suggestion:

if (stream != null) {

readFile(stream);

}

Form of The if-else class of statements is incorrect.

Suggestions:

if (condition) {

statements;

} else if (condition) {

statements;

} else {

statements;

}

Too Much space is added.

Suggestion: Remove unnecessary spacing to improve readability of codes

isExited is grammatically incorrect

Suggestion:maybe you could use isExit as this will sound more like a question and it fits the English grammar better

Great Job. It follows the coding standards for loops and indentation.

Naming of variable is abit unclear here.

Suggestion: Make this variable to be name more to its function , more towards what it is being used.

Maybe this section could be refactored into a sub-method to make it shorter?

I would use a getter for the attributes of subclasses of Task. 😃

I like your way to refactor the printSeparator() function

How about using a constant MAX_TASK_NUM for the array length.

I like this refactoring 😃 BTW, how about giving a more intuitive name such as printSuccessfullyAddedMessage()?

Is there any reason for you using print() instead of overriding toString()?

I like your treatment for this special case.

Maybe this part could be refactored to a sub-method?

I like the startsWith(), which is a neat method to detect the command

I would set a getter for attributes of subclasses of Task 😃

I like your way to refactor the messages 😃

I like how you treat TaskList

Looks Good, but you could declare this as a constant?

Avoided deep nesting. Well done!

I like that you used comments to explain the code, but perhaps the comment could be placed on top of the code itself?

I like how you declare a constant to avoid magic number!

I like how you made your code very easy to understand by having only functions in main!

Followed constant naming conventions. Nice!

Nice that you use a function to set tick, but perhaps this function should be named setCompleted() or setTick()?

Nice that you use a function to set tick, but perhaps this function should be named setIncomplete() or setCross()?

Nice that you used Egyptian style brackets, but perhaps there should be a space before the bracket so that it looks more standardized?

Good that you list the imported class explicitly. Well Done!

Nice that you used Egyptian style brackets, but perhaps there should be a space before the bracket so that it looks more standardized?

Comments are indented relative to their position in the code. Well Done!

Perhaps a more intuitive variable name here for 'desc'?

Perhaps these can be extracted out if not in use? You can consider storing these comments somewhere accessible if you want to refer to it in future, but I think it would be better to extract them out now to make the commits in your pull request more readable!

Should the tick be made into a constant to avoid having magic literals in your code? I noticed that there are a few more occurrences of this in your code elsewhere.

Perhaps you can use a more intuitive name for "descArr"? I noticed the same issue in several other variables too.

Perhaps you can consider refactoring some parts over here? Considering this is the main method, I think it would be better if it could be made more readable such that readers can quickly understand what behaviour the code will give when the program is ran.

Should the case statements be indented to the same extent as the switch () statement?

Should this be @override instead? If you think so too, then it would be applicable to the other overridden methods that you have!

Perhaps you might want to create a variable for inputArr[0], such that it would be clearer for the readers and yourself as to what you are checking in your switch statement?

I think the names you are using for your methods are good and intuitive! You have also named them in camelCase too which follows the coding practices!

I like that the naming you used for this (in particular taskType) is intuitive, despite passing in 'command' in processUserInput.

I think it would be better if comments are added to the methods to explain what they do. I noticed the same issue in the other methods that you declared too.

Should this be split into two lines?

It may be neater to refactor this into another method

Same here. Refactor 😃

And here 😃

Yup refer to https://se-education.org/guides/conventions/java/basic.html for coding standards

I like how you used toString()

This looks really neat! good job

Will it be better to use toString() instead of printTask()

Would it be better to have the function name as isDone() ?

I like how you've made a different method for every different command 👍

Nice idea to detect errors 😃

Not sure if you should use magic numbers, maybe try declaring a constant?

Not sure if methods should be named in continuous tense

Perhaps can change the 100 to a constant "MAX_TASK" to avoid the magic numbers

Perhaps can name the Task array to "tasks" instead of "list", this name probably will be easier to understand by other developers

Perhaps can extract this to a method outside of the while loop so that the level of abstraction inside the while loop is consistent.

I like how you add an additional comment here to annotate the function of this code block.

Perhaps can add a few more variables to store the processed message before this line of code. This can decrease the complexity of the expressions and improve the readability. I noticed the same issue in several other parts too.

Similar to the above line of code where you created new tasks, i.e. "list[count] = new Deadline", perhaps can try to avoid the complicated expression

Perhaps name this class to "Todo"? so that it is more similar to the related subclasses (i.e. Event, Deadline)

Perhaps can refactorize this part of the if statement to avoid deep nesting

I like how you have no indentation for case clauses.

Perhaps can declare the maximum size of the array list?

Perhaps can change the "[D]" to a constant so that other developer knows it is for deadline type. I noticed the same issue in other subclasses too.

Should this either be a cross/tick?

Good use of SLAP, the main() method is very neat and readable.

Since all classes in Java inherit from the Object class (directly/indirectly) it would be a better practice to include @\Override annotation when overriding the toString() method because the default toString method of the Object class prints [ObjectName] + Hex Code.

Remember to put curly brackets to wrap any number of statements in an if-else block. Even if it is just 1 statement.

Perhaps this could go under a function called addTask(). Will make the program more readable and modular.

Since all these values are immutable and basically constant in the program- they can be declared as 'final.'

This part could be refactored to a function to check for validity of operation.

The println() lines are quite long, consider line wrapping the code.

if-else blocks should ideally have brackets wrapping the text even if it is one statement only

Should be defined as a constant using 'final.'

The name of this integer could be more descriptive.

This variable could perhaps have a different name - because it stores the number tasks. The name 'size' is a little less descriptive in the terms of the purpose of the variable.

Since these functions are creating Task objects, it would be better to name them according to the their purpose. That is, something like createEvent() or addEvent().

It would be better if you could be more specific on the names of variable such as index and index2.

Consider creating a function for the line "____________________" so that it will improve code clarity and readability

I like the way you specify each import class

I would suggest if you could create this into a function as well as it will improve the readability

To add on, i suggest you rename your objectname to something more descriptive/readable such as "task"

It will more appropriate if you could indent this line into 2 separate lines

Recommended to have the else-if on the next line

Well done in writing individual import statements

Consider elaborating your variable name to something more specific such as taskindex

Nice way of displaying these messages by just calling a function

Do take note of the "{" spacing after the for loop ()

Code Quality: are there more than 4 levels of indentation?

Code standard (Naming): should greeting() (the method's name) be a verb?

Naming: not very descriptive names

Coding standard: should there be spaces in between?

Coding standard: should there be spaces before '=' and '(true)'?

Coding Quality: maybe it can be explained why 5 is there

Coding Quality: is this method used anywhere?

Readability (magic number): maybe you could explain why it is 9?

Coding standard: should else if be right after the above bracket?

should else be right after the above bracket?

is the line quite long (>120 characters)?

Maybe this icon can be a named constant to be more descriptive

I like how this function captures all the welcome messages

I think the case statements should be at the same indentation as the switch word

Maybe these print statements could be refactored into a function as they seemed to be used a few times

I noticed this issue in a few other places too

These indexes might be a little hard to understand as magic numbers so you may want to make a constant to describe them better

Same thing for some of messages that are printed, might be better to make them constants?

Maybe this variable can be described a bit more?

I like how this boolean is named

Maybe this can be task instead of t, what do you think?

I think we should try not to use arr as it may be too general, maybe something like commandWords instead?

It will be better to create another method called donetask and put your code inside the method

it will be better to create a todo class and put the behavior of todo inside the class

could you let the use of temp2 be clearer by changing its name?

It's very good to set TaskList() as another class so that can avoid Duke class being too long.

Besides set the border as a constant string, it is also okay to create a printline() method, it might also be easier for you to add \n at the end of constant string if java allows

for the methods which has only one line, it will be also okay to directly include it in the if statement, because I find it a little bit hard to read if we need to switch to different methods frequently.

for the methods which has only one line, it will be also okay to directly include it in the if statement to make it easier to read.

Overall the coding standard is great!

Will it be better to give the constant 100 a constant name like MAX_TASK_NUMBER?

The else if need to be in the same line as the end bracket "}" so that you can meet the requirement of coding standard

it could be clearer for the code by keeping your format consistent, the upper line could be [taskNumber - 1]

Applying SLAP would make main function more readable.

An example is you could refactor the switch into a method call executeCommand().

Is it possible to remove magic number, try declaring "/by" as a constant DELIMIT_DEADLINE.

Perhaps you can refractor the number 100 to show the meaning of it since its a constant?

The description can be changed to follow the standard coding format.

Use this for reference: https://se-education.org/guides/conventions/java/basic.html#comments

And a quick tip within Intellij "/***" + enter will help with the description formating.

Perhaps the method meant printInvalidCommand(), as suggested by the function of the method.

I think the naming of the method can be improved, as the exit program would suggest the method ran System.exit(0);

Please use all caps for Constant (final). e.g. SCAN

A reminder to change "\n" to System.lineSeparator() which is more appropriate. If you find it hard to use consider making it a constant. Example: private static final String LS = System.lineSeparator(); and printing LS when line separator is needed.

I'm not sure if makin a class for printing greeting is actually needed. Consider just putting it in the main as constant and using a method to print it.

Try using class level members to store the number of tasks and the remaining number of tasks.

Do refer to the textbook: https://nus-cs2113-ay2021s1.github.io/website/book/cppToJava/classes/classLevelMembers/

Perks of doing this, you can increase the number of tasks within the constructor and the remaining task can be computed when you make a task as done.

The naming of the class usually will not be an action. I think such a class can be simplified into a method within the main.

Coding standard violation: the else if should be in the same line as the end brackets.

You may refer to the link below:

https://se-education.org/guides/conventions/java/basic.html

Hi, I think there is some extra empty line here. You might want to edit it.

Avoid unnecessary use of this with fields.

Refer to: https://se-education.org/guides/conventions/java/index.html#variables

I think this is a coding standard violation. Please refer to the following example from: https://se-education.org/guides/conventions/java/basic.html

E.g:

while (!done) {

doSomething();

done = moreToDo();

}

Same here, please refer to the following example, (i think there should always be a space before and after the parameter) :

if (condition) {

statements;

}

well done, no coding standard violation

Hi, is this considered unnecessary use of this with fields? Correct me if I am wrong.

The spacings between the methods are not consistent.

perhaps the 100 can be made into a constant? So that it is more meaningful instead of just having 100 as a magic number

It would be good to convert the 100 into a constant, make it more meaningful instead of just having a magic number

good job on having the consistent layout for Task.numberOfTasks - 1

Consider using a constant for the max number of task?

Maybe consider separating code to different functions done(), list() to improve readability

I think you can afford to be more descriptive for the variable tmp. I understand what is echo but tmp is a bit ambiguous.

Maybe consider trimming also in the case that the user inputs an extra space between the event and the description

All of the other classes (Task, Events, Todos Deadlines) are all very readable and within coding standards. Great Job!

Perhaps considering renaming classes to either all plural (Tasks) or all singular (Todo, Event, Deadline) for more uniformity

Maybe consider using string.format to reduce the amount of concatenations required?

Great Job at coding standards over here! Although maybe consider moving the public final constants to main and making them private.

I think this variable is a bit ambiguous. Maybe replace with remain or stay, to avoid confusion with the EXIT constant

I think the line spacing between the statements can be removed

do {

statements;

} while (condition);

Nice separation of the if statement to improve readablity!

I think these two final ints/constants should be capitalized with underscores

Perhaps a more intuitive variable name here? for example: itemNo

I really like how you divided the tasks here! Good Job 💯

Maybe this comment and similarly line 40 can be removed, since they do not really explain the context.

Maybe the initialization variable could be shorter, for example int i, as this is allowed for for loops

should this be written in the following form instead?

for (initialization; condition; update) {

statements;

}

Any reason why this package is included here?

Maybe these can be put in different functions for different types of inputs by refactoring the code? It might help to make the code more readable.

Nice! I like how you have used white spaces to enhance readability. 😄

Should the sub string be stored in a separate variable instead, to avoid reusing the variable "line" for a different purpose? I noticed a similar issue in other places as well.

Maybe, this be separated out in a function to follow the SLAP principle. I found similar instances in other places as well.

Perhaps a more intuitive variable name here?

Great Organization of the code.

Good job explaining the commands

Modular code is more readable. Nicely done!

Variable name could be less ambiguous

Great use of error handling and making the program as user friendly as possible. Perhaps in future iterations this could be made using exception handling to improve code structure.

Great addition to the code on your end.

A switch case statement might have been better suited to help make the code a little more readable

Good job making this a separate function.

Could consider making task and abstract class?

Consider removing this space

Consider removing extra lines here

A placeholder for a first review

Great job capitalising constants!

instead of line,task list is suggested to be more clear about where this list belong to

Command might not be clear enough that this comes from user. Perhaps something like "userInput" will be clearer

if-else used correctly, but a switch statement might be clearer in showing the consequence of different inputs

methods are generally abstracted, which will improve readability further down the road. good job

Just commenting that the use of a switch over if-else statements means that each case is more readily shown upon review. good job

function name greeting should be imperative such as "drawHorizontalline" instead of "Horizontalline"

methods used are generally abstracted, which improves readability later on. Great job!

Using a case instead of else if helps improve readability further down the road, good job!

Since the ListOfInputs is a variable name, it should be in camelCase.

The if-else statement generally follows the standard except there are extra lines between } and else.

The statement seems to be a little bit too long and complicated. You can consider shortening it by defining new variables to hold some values

It's wise to define constants in a separate class clearly.

The statements here are also a little bit too complicated.

nice nested if statement

Here you should leave a space between ) and {

a space between ) and {

I like your nicely structured nested if statement

You should leave a space between ) and {

May I suggest 9 to be refactored as a constant? If I am not interpreting the code wrongly, I suppose it can be refactored as MAX_ARG_NUM or MAX_ARG_COUNT.

I would like to suggest changing listTasks() to showTaskList()! I believe this alternative is clearer on task function since list can refer to both list (noun) and list (verb).

Good implementation of the switch statement to handle individual states of operation!

Correct me if I am wrong, but I think the spacing on both ends of the sign is not necessary. Otherwise I will suggest it to be standardized with the rest of the mathematical expressions.

I appreciate the naming conventions for the functions. It helps me get a strong general idea on what the function does before I dive into the code!

Yes Randy. Its is a minor formatting issue that I have found, if it is at all an issue in the first place.

Another suitable control flow alternative would be the switch statement!

I think it is possible to simplify this even further by applying the condition to whether to append an "s" or a "\0" following the task word and reducing it to a single line of code, but that's just my preference!

i< tasks.size() ------ the spacing surrounding the '>' operating seems to be different from the '-' operator. Perhaps it is a good idea to standardize the spacing!

Since the eventTag is associated with all tasks, perhaps we can put that as a private attribute associated with the subclasses of Tasks? I would suggest you introduce it as a class attribute of Task and assign it with the corresponding tag in the constructors of the subclasses.

Consider using constants to store the commands.

I like the use of constants to refactor these magic strings!

The use of switch statements for commands makes the code look very neat! 👍

Consider changing the indentation here to follow the coding standards regarding wrapped lines (i.e. "Indentation for wrapped lines should be 8 spaces more than the parent line.").

Consider changing the variable name to follow this naming guideline: "Plural form should be used on names representing a collection of objects".

Good job following the SLAP principle in your main method!

Consider adding spaces between the + operator here, it seems to be inconsistent with the rest of the code.

Consider adding spaces between the + operator here, which seems to be inconsistent with the rest of the code.

Consider renaming this function to printSeparator() to follow the guideline that methods should be verbs.

Consider using a more descriptive method name, for example listTasks() to show what you are listing.

Perhaps declare a final variable for the maximum number of tasks, as follows:

static final int MAX_LIST_COUNT = 100 ?

Maybe extract this as a constant, like so:

public static final String BORDER = "____________________________________________________________\n"?

Perhaps extract this as a greeting method or constant to better align with SLAP paradigm?

Maybe this need not be declared here, since you don't use it outside of the main while loop?

Perhaps "command" is not the best name for this variable, since in your main while loop command is assigned to the whole line from read.readLine(); would it be better to name it something like userInput or userInputLine instead?

Apart from extracting as method and overall improving readability, would it be better if Integer.valueOf(arrOfStr[1]-1 is assigned to a variable before accessing it twice, on lines 38 and 41 as such? I think it would improve readability, especially if the variable is named appropriately.

Perhaps can declare a final variable for COMMAND_BYE and the other similar commands, as in

static final String COMMAND_BYE = "bye" ?

Otherwise, clean code!

Might this be better/easier to follow if not a fall-through? Like

case COMMAND_DEADLINE: if (!line.contains("/by") { throw new DukeException("☹ OOPS!!! Missing or incorrect /by statement"); },

and similar for the COMMAND_EVENT, then followed by the fall-through case of

return line.substring(line.indexOf(' ') + 1, line.indexOf('/') - 1);

Otherwise, super good code and readable, easy to follow, and very logical.

Perhaps can extract as method for these two lines, to better align with SLAP paradigm?

Maybe can separate the printing done in addToList method with the action of adding to the TaskList? Like one call to add task to TaskList data structure, and another to print that the task has been added to list?

Should the size 100 be extracted out as a constant?

Perhaps the condition statement should be the input line begins with done, not contains done?

Is 5 considered a magic number here?

Is 9 considered a magic number here? A similar problem also happens in the next else if part.

Does e seem to be meaningless as a variable name?

Is a meaningless as a variable name?

I like the way you distinguish different commands!

Should the size 100 be extracted as a constant?

Perhaps use the name "words" rather than "word"?

I like the way of judging the conditions of "done" command!

You can consider refactoring your strings as constant instead, such as MESSAGE_GREETING, would make the code more readable and better for UI/UX design in the future.

I would suggest your brackets follow the K&R style brackets (Egyptian style) to follow with the Java Coding standards

Maybe you could add some Javadoc comments to improve code readability

I would suggest changing the Tasklist array name to follow the Java coding conventions of naming, - camel case and arrays should show that they contain multiple elements, such as tasks

Suggest to use egyptian style brackets instead

I would suggest to follow the Java coding standards for white spacing, where there is white space after each operation

Hi, I would suggest refactoring the strings into a constants, such as MESSAGE_INVALID_COMMAND, instead of putting the string directly into the printline statements. This would improve readability and improve the UI/UX design experience.

Maybe you could change the name from myObj to something else, such as myCommand

Maybe you can put the long if/else chain into a switch statement? May improve the code quality

Maybe you can refactor this into a separate method, so you can reuse the code for both event and deadline methods. This may improve the code quality

Can try changing all the quotes of underscores into a predefined constant, to ensure no magic literals kind of thing?

Note that your case should be in line with the word switch (I think)

Correct coding standard, no violations as such!

Correctly indented, with capitalisation requirements followed where needed.

Coding standard followed well in this error handling portion of the function! Nice.

The blank line may not be necessary, but noted that you did this once in another add function. Also noted other functions with print lines don't have the blank line between the print section and the rest of the function. Maybe can standardise no blank line?

Meticulous commenting, and proper indentations and spacing. No coding violations, very high readability. Nice!!

Indented really well, which really helps in fluid reading and comprehension of your code! Nice.

Solid comments that clearly indicate what the function intends to do, and also neatly indented. Readability wise, all good! Naming convention adhered to as well!

Neat indentation, and informative commenting that explains function use well. Naming conventions all followed! Good job!

Import Statements are listed explicitly, which is a good practice!

Consistent Indentations and spacing, good job! Simple to read and understand

Excellent practice on having consistent coding standards across the different class files

There is no content inside this method, maybe there was an oversight on this? Perhaps a removal of this method might reduce confusion

Maybe you might want to remove these empty line spacings that are found in your methods? It might allow easier flow of readability by the reader.

Inconsistent spacing and indentations make it difficult for readability. Maybe altering it to be consistent will help with over flow.

Good work splitting the lines to make it easier for readers to view your code

Do use K&R bracketing style, even if it is only one line of code under the if statement

Try to eliminate all these empty spaces as it makes reading the code difficult

Try to keep all your spacings consistent, even with normal statements.

Maybe you can shorten this line? The hard character limit is 120 characters. It seems like it occurs other times too.

It seems like your comments have inconsistent indentation!

Why is the break here commented out?

Very nice! Proper formatting of if-else statements here.

Maybe find a way to shorten this line? It seems too long.

Perhaps you could declare a constant for the line?

Maybe give the task array a clearer name?

It would be very helpful to have a comment here to explain this line!

Maybe declare the variable inside the for loop itself?

Maybe refactor these print message lines across your multiple add task methods?

Should these be declared as constants instead?

Should the formatting here be modified such that there is an extra space between the end of the parenthesis and the opening brackets ){ vs ) { , as well as between the end of the bracket and the start of an else if?

Is this import being used?

I noticed you use the optional @Override for every single overridden function except for this one. Should there be one here too for consistency?

Should taskCount++; be put in the update as the 3rd parameter for the for loop since its the iterator for this loop?

Should there be an extra space between while and the opening parenthesis and then another space after the ||?

Maybe you should consider making this a static constant? It would help tidy up the repetition in the code.

This line is 122 characters long, should this be split into two lines instead?

Again, should this be a static constant instead to help tidy the code up?

I like the use of UTF-8 encoding here since it addresses an obvious issue in the output files

The indentation seems a bit different from the Duke.java file. Did you use 8 spaces instead of 4 for this file?

I like how you separated the cases between one task and multiple tasks

Should this be extracted out? I notice the same issues below.

Can this line (120) be replaced with addTask method from line 43?

I think you can consider splitting the string, e.g. TASKNAME -> TASK_NAME to improve readability.

This return statement can be removed.

Are these strings going to be modified during runtime? If not then you can consider changing this array of String to a constant.

Shouldn't this part be extracted to a method under Task class?

I think you can consider using switch() instead of multiple if statement to improve readability.

a small typo here

Can add functions to do printing to make the main shorter.

Can remove redundant code here.

Maybe can name as markDone instead?

Looks good!

Maybe "at" instead of "At"

Maybe instead of "Message" can use "Description".

Same for this. Maybe instead of "Message" can use "Description".

Maybe instead of "temp" can use other words?

Good job on the refactoring!

Instead of state maybe can use "command"?

Seems to have an extra line here?

Would be good to include a space after the "for"

A space after "if"

space after "if"

space after "else if"

Space after "else if"

Space after "if"

Would be good to include a space after "for".

I like how you declare your messages as constants with appropriate naming practices.

I like your naming of the various methods

I like your indentation on the case statements

I like your usage of parenthesis

Should you declare these strings as constants?

Should these be extracted as a separate method to initialize the variables?

I like your use of SLAP

I like that you named the icons

Since you declared the icon as a constant already in the other class, should the icon here not be used as a constant a well?

Can the separation of the description and the date be done together in a different method?

Should this number 4 be refactored to a constant to avoid magic number?

Perhaps this part can be refactored to enhance readability.

Perhaps this number 4 can be refactored to a constant to avoid magic number.

I noticed the same 4 space strings are used to indent your output in the terminal in several other places too. Perhaps you can consider introducing a constant to enhance readability and ease future code maintenance.

I like how you created another getTypeIcon() method.

Should this else if statement be on the same line with the brace above to follow coding standard?

Perhaps a space between switch and (command) here?

Should the opening brace after case LIST_COMMAND: and the closing brace after that be removed to follow coding standards?

Perhaps the opening and closing braces of the default clause should be removed also to follow coding standards.

Should the comment //return tick or X symbols be moved to a new line before return (isDone ? "\u2713" : "\u2718"); and changed to // Return ... to follow this convention?

function name greeting should be imperative such as "greet" instead of "greeting"

instead of naming it list, name it "taskList" so that when you create multiple lists, it is clear what each list is for.

instead of line, i suggest "userInput" to let people understand that the line variable is suppose to contain the user input.

num should be more descriptive as to what number is it refering to.

function name: Status is still too ambiguous. The icon is specifically refers to if the task is done or not.

These 3 lines are not on the same level of abstraction.

This still involve lower level details, the only part that is being abstracted is adding the ToDo to the task list.

Should abstract the creation of the ToDo object and the splicing up of the inputCommand.

Same comments. The function addTask has not adequately abstracted away the details of the whole process.

This function does 2 things. Mark task as done, and print out a bunch of messages. It improves readibility if you could

abstract these 2 functions.

I like how you defined a constant variable instead of using magic literals.

I like how you followed "SLAP" (single level of abstraction per function) in your main function. All the methods in your main function have the same level of abstraction.

I like how you used switch here and that you had "break" after each case statement.

I like how you made your method private to prevent other classes from calling it.

Should variables be named using camelCase?

Should the word "else" be put at the same line as the "}" for if-part?

I like how you commented for the method.

I like how you separated one long sentence into two lines.

I like how you listed the classes imported explicitly, instead of using "import java.util.*"

Consider having a space between () and {

Good naming of boolean variables. Alternatively, consider using shouldAddTodo etc.

Good use of indentation for long line

Consider renaming to goodbyeMessage, as goodbyeWords may sound like it's referring to an array of words

Consider removing the Messages. prefix, as it is not needed for a method in the same class

Consider wrapping the whole if-else block in a try...catch and then use case/if-else to handle the various exceptions. This reduces repetitive try..catch blocks for the same error.

Consider creating String lowerCaseInput = input.toLowerCase() to reduce toLowerCase() method calls

Consider renaming to markAsDone

Consider renaming to getTaskArray or getTaskList, to make it clear that the return of type ArrayList

Good use of comments to explain what the program is achieving!

Second line of wrapped line can be indented 8 spaces in compared to the first line

The second line of a wrapped line should be indented by 8 spaces

Good use concept of abstraction!

For wrapped lines, it is recommended to have 8 indented spaces for the second line onwards

Can make the bool variable more like adjective words (eg: isEndConvo)

Can add comments to explain what each function is doing

Eg: to set the meeting time of the Events

Have the second wrapped line being indented in by 8 spaces

Can further reformat this

Eg: put this in another method

This statement is quite long, maybe could use some refactors to make it shorter.

Maybe this could be refactored with some other print statements to a function so that do not need to write this long statement everytime.

I think these methods fullfill the requirement quite well.

I like this switch statement since it makes the code clean and short.

I think your this class has fullfilled the requirement quite well.

Maybe this could be declared as constant or maybe could be refactored with some other print statements to a method.

Maybe toString() is a more suitable method than print().

Maybe these addSomething can be refacored to make these shorter. But the naming style is easily understood by others.

I like these refactorings which make the code quite clean.

Try splitting your main() method into more methods to improve readability - makes the code look less chunky

Try using more explicit variable names so that readers can identify them in the code better

Perhaps you could refactor the main method into a few more smaller methods to make it less chunky

Try using your printLine method here instead

Try to use more explicit variable names to improve readability

Try splitting the horizontal line as a method to improve readability

Maybe can use other words than temp to name variables

Perhaps you could split the main() method into more methods to increase readibility

Maybe you could use a more specific name than line

It would be better if there was spaces between i = 0 and i < taskcounter

Adding some empty lines here allows for better readability

You may want to use String.format to allow for better formatting of output

Comments should be in the same indentation as the line below

I think it would be better if there is no empty new line here

I think it will be better if there was no empty new line here

There is another empty line that can be removed here

Might want to consider printing the stack trace to handle this exception

Name of error can be slightly more descriptive

I think you could initialize as "ArrayList<task\ cwf="C:\repos\nus-cs2103\dashboards-base\contents\cs2113\ip-review-comments-panels.mbdf"> taskArraylist = new ArrayList<task\ cwf="C:\repos\nus-cs2103\dashboards-base\contents\cs2113\ip-review-comments-panels.mbdf">();" to be more clear. </task></task>

I feel that this part could be broken into different function/class for better abstraction

Perhaps you could use different name to better differentiate command and commandSubject

Maybe replace "line" with something like "userInput" ?

Instead of multiple else if, you can consider using switch case to replace it

I realised you have declared "int divider = line.indexOf(" ")" multiple times in different else if blocks. Perhaps you can declared only once at the top

I think this if else block is doing the same thing in both cases

I feel that breaking some parts into smaller functions will improve readability

you might want to use taskList instead of list when there are more lists in the future

I think you should define constants like "list" or "done" in an enumeration.

I think you should define constants like "list" or "done" or "bye" in an enumeration

Should the variables be named something like "deadlineDescription" and "deadlineTime" instead of "what" and "when"? The naming is quite hard to understand. Thank you.

Should this be named "taskList" instead just "list"? "taskList" seems to be in sync with your counter "taskCount" and so on.

Should the variables be more descriptive by being named something like "eventDescription" and "eventTime" instead of "what" and "when"?

Should the variables be named "taskDescription" and "taskTime" rather than "what" and "when"?

Should command words like "bye", "list", "done" be defined in an enumeration?

Should command words like "todo", "deadline", "event" be defined in an enumeration?

Should this variable mention something about time? eventAt may have two meanings 1) location of the event 2) time of the event.

Perhaps these 4 lines can be combined for conciseness?

if-else statements may be changed to switch statements

Maybe at variable can be more descriptive? e.g. eventDate, atDate

"[" + "D" + "]" can be combined to be "[D]"

Any reason why you created a new Task instance instead of changing the isDone attribute to true?

"["+"T"+"]" can be combined to be "[T]"

"[" + "D" + "]" can be combined to be "[D]"

Should this be [E] for event instead of [D]?

Perhaps horizontalLine() method can be named such that it is a verb? (e.g. printHorizontalLine())

Hi, your code looks neat and easy to read!

This number "4" is a magic number and should be converted into a constant.

The same goes for the magic numbers following below.

There's quite a few occurrence of this line of code. Perhaps it could be refactored out into a separate method.

Hi, your main code looks really neat and easy to read

You might want to shift some of the message codes into another class called messages so that there would not be too many function in a single class. I think this would help greatly with code readability

Hi, your code looks good and easy to read

I think this line should be should be wrapped into 2 different parts instead of being in one line

I dont thinks the use of "this" for this.isDone is necessary and can be replaced with just isDone = false as isDone is not shadowed by anything

I like how you created constants for these Strings. The naming convention for these constant variables are also correct and consistent. Good job!

Perhaps for this magic number change it to a constant that you can edit easily

The spacing of { brackets are inconsistent here.

Im not too sure what the 0 and 1 is referring to in argArr array. Perhaps refactoring away this magic number might improve code readability.

Great use of comments!

Good job importing only what is needed

Some magic numbers here. Not sure if its better for you to change it

Great use of comments

The while loop could be too extensive and hard to read. Do consider changing the lines of code into methods such as commandDeadline. And on top of that, the while loop could be refactored into another method eg executeCommands. 😄

Neat form of if-else statements! Good Job!

Just a small change here: Boolean done should be named isDone

Good job on the explicit importing of the specific classes.

Just a small indentation error here. There is no indentation for case clauses. https://se-education.org/guides/conventions/java/basic.html 😄

Could consider changing the function name to printWelcomeMessage()

Could define the magic number here 😄

Could remove some uneccessary blank lines

Accurate use of PascalCase! 😃

similar for all the variables, eg NumberOfTexts should be numberOfTexts

Can consider to delete this extra unnecessary blank line so that code is neater and of better quality.

Similarly for the rest of the code in the other classes.

Consider spelling out arrOfStr fully as it may vague to arrayOfString to make code more readable.

Similarly for the other variables in the other classes, would be good if you can spell them out fully.

I think there is a need to put a white space between "do" and "{" to follow the coding standards.

Similarly for the "if" and "while".

I think this condition might be too long and may be hard to read.

I think it is possible to put it at the else part of a if-else clause so that you don't need to write this long condition out.

May be better to indicate what does i stands for.

May be better to spell out number, similarly for the other variables that has "num"

Consider putting a white space before and after the "+" for better readability, similarly for the rest of the code

Coding standard:

The if-else statements are adhering to the recommended coding standard however I feel that there might be too many nested if-else statements, perhaps a switch-case statement would aid user readability and easier debugging? The incrementation of taskCount in each if-else next perhaps can be refactored out to avoid duplication of code.

Great use of class inheritance. Keep up the good work!

Coding standard:

5 is indeed a magic number here. Perhaps you could elaborate more about the number with a comment? Otherwise, perhaps use a constant with a sensible name like STRING_POSITION so that reader can follow your train of thought.

Coding quality: I feel that there might be too many nested if-else statements in this part. Perhaps it'd be better to avoid arrowhead style and substitute it with switch-case statements which would improve code readability and understandability.

Code quality: the operation countThings++ is repeated, perhaps it can be further refactored to minimise code duplication.

Good job on following coding standards where an explicit "//fallthrough" comment is provided in the case BYE when there is no break.

Good job on structuring if-else statement this way! Very easy to follow and understand.

I like the happy path structuring here. Very easy to follow and allows unexpected condition to be easily spotted. Good job!

Perhaps you can consider moving line 30 up to line 29.

Perhaps you can consider leaving a white space after ) and before {. Same for the other methods in this file.

Hi! Perhaps you can consider giving this a more specific name in plural form.

Maybe you can consider breaking this segment of code into smaller methods for better readability.

Maybe you can consider putting these into methods for better readability.

Hi! As this is a method, perhaps you can consider adding a verb like show or print to the method name. You can also consider this for your other methods.

Perhaps you can consider leaving a whitespace between ) and { to make your code more consistent and you can also consider this suggestion for other methods as well.

Hi! Maybe you can consider removing these additional blank lines.

It would be better to state what exactly you are importing instead of .* for example import java.util.Scanner so we know what exactly it is that you imported and will be using

I think this could perhaps be declared at the very top.

I think you might have forgotten a whitespace next to the bracket and break keyword;

Likewise as above, the whitespace is missing and there are a few such cases below as well

Seems like in some cases you leave a line between methods and some others you don't. Perhaps it may be better if it were more consistent

I think perhaps currentTime or something to that effect might be better in terms of clarifying what the current variable refers to.

Seems like your comments for the above methods were all written in the line above the method. Perhaps to be consistent, you could also have this comment in the line just above.

I see the index was declared here, but only used in the else statement. Would it be better if we declared it over there instead?

Correct naming convention used for constants but perhaps the final keyword should be used for these constants?

I agree. It would also help to make the code for this method shorter and easier to read.

Should a getTotalNumOfTasks() method be used here instead of directly accessing totalNumOfTasks?

Good use of named constants to avoid magic numbers and improving readability.

The code is well written and although variables and method names are clear, maybe adding comments could make the code easier to understand.

Good to see that the correct naming format is being used for constants.

Nice alignment of the case labels to the switch statement according to the standard.

I like how you place the array specifier immediately after Task to indicate that the type of the variable is an array of Task type elements.

Maybe can use switch to replace the else-if set.

better to use scanner instead of '*'.

Maybe switch-case can be used instead of the if-else statement to make the code reads easier.

May consider write a method to print messages in each case so the main function body can be shortened and the logic can be easier to understand.

May consider using "dueTime" instead of "by" to make it easier to understand.

Maybe use "printLine" instead of "printLines" to make it more precise.

Can add "@Override" here to point out the method override.

May use addTodo instead of addToDo.

I like how you made use of the parent's toString()

Should this be named as HORIZONTAL_ROW? HR seems a bit vague and might be mistaken for e.g. hour.

Should these variables be private since Duke has no subclasses?

I like how you explained the function and its parameters 😃

I like that you explained the status icons! 👍

I like how you handled the case where the list is empty! 😃

I like how you are very detailed in validating the inputs! 👍

I don't quite understand the purposes of these variables. May I know what are they used for? Also, I feel that the name is a bit lengthy, and the grammar is a bit off; for example, it should be REQUIRED_NUMBER_OF_ARGUMENTS_FOR_TODO.

I like these public static final Strings!

I like the way you refactor the things in the while loop, which makes it clear for me to understand

Should it be Deadline extends Task? or is it ok to just write Deadline.

These methods are very useful to check some exceptions

I think you can use uppercase for the case statements

Overall the code is very clear, readable.

Maybe you can use some methods to print the horizontal lines and other messages.

Good use of trim() is good, I like it.

Coding standard violation: line length exceeds 120 chars. Do a line wrap with 8 space indentation

Coding standard violation: names representing methods must be verbs and written in camelCase.

Can consider a more reasonable name, such as printBetweenLines, to better explain what the method does

Follow the standard way to add comments. For example:

while (true) {

// Do something

something();

}

Avoid using magic numbers. Name the constant something like MAX_TASKS.

Methods should be verbs.Can consider a more reasonable name, such as printBetweenLines, to better explain what the method does.

Good usage of SLAP that makes the code looks neat and readable. Great job!

Very helpful comments that explained the code in details to anyone who reads it. I really like that, well done!

Great job making sure that all possible scenarios are dealt with. I'm not sure if I missed it, but perhaps you can consider the case of trying to do a task that is already done? It's not exactly an error but it would be great if your code recognises that and acknowledges it too.

Code is very organised and easy to understand

The else if and else statement format are not abiding the java coding standards. There are also several unnecessary empty lines in the while loop, consider removing them.

Unnecessary empty line at line 24

Code is very organised and easy to understand

Code naming is good and in accordance with java coding standards.

The while loop is a little long and affects the ease of readability. Consider shortening the loop or separate some code into functions/classes.

Try not to have nested loops (AKA arrow head) in your code. Try to refactor some parts of this loop to prevent this from happening.

I think these constant names should be in uppercase, what do you think?

I think it will be better to include curly brackets and put the statement in a new line even though there is only one statement, what do you think?

You might want to include a space after each punctuation in the for loop so for example, for(initialization; condition; update), there is a space between ; and condition as well as ; and update

I like how you combine the greeting messages into one function printWelcomeMessage(). Keep it up.

I think these print statements can be refactored into one function on its own, similarly for lines 70-72 etc, following SLAP (Single Level of Abstraction Principle), what do you think?

I think these statements can be refactored into another function such as insertDeadline, what do you think?

I think these statements can be refactored into a function such as insertEvent, same for the statements in Todo tasks in line 155 to 159.

I think that the naming for String s can be changed to a more descriptive name such as taskToAdd, what do you think?

Maybe this can be in caps since it is a constant

Avoid unnecessary use of this with fields.

Refer to: https://se-education.org/guides/conventions/java/index.html#variables

I like this too!

else not on the same line as the brace, violates the coding standard

Empty constructor

Avoid unnecessary use of this with fields. There are a few occurrences.

Refer to: https://se-education.org/guides/conventions/java/index.html#variables

Try to avoid deep nesting

https://nus-cs2113-ay2021s1.github.io/website/schedule/week4/topics.html#w4-6-code-quality-readability

Maybe you can try to shorten this method such as through refactoring

Perhaps consider extracting this out into a method.

The name for this method could be renamed to printList to help ensure consistency with printTaskAssignment

It might be worth extracting this out into another method too.

Is it possible to relate these magic numbers to "deadline" and "\by" somehow? Just to make it clear where they are from

Could these magic numbers also be changed to relate to the strings as well?

Could this be renamed as tasksList? It might make it clearer it is to store multiple tasks

Could this be renamed to printWelcomeMessage? Just to make it extra clear it is to print a message.

Consider just using an else statement if there are no more conditions to check

I like the use of @params and commenting every single function

Perhaps some of the comments such as these could be removed, they are quite self documenting.

Nice clear separation between variables and constants

maybe consistently add or remove this blank line, abit inconsistent in other files

woud be nice for consistent spacing around the equal signs

this function is a bit long, perhaps extracts some lines for each case as a separate function

Maybe rename args in the function argument to this function as a "description" instead of making a new variable to copy it

Maybe change list name to taskList

Maybe you can refactor this out of your main as a printWelcomeScreen method?

Your code is generally well written and very readable. Keep it up!

I like that you used constants for the Strings which will make your code more readable.

I think your if-else statement here should follow the required Java coding standard.

https://se-education.org/guides/conventions/java/basic.html

Your if-else statements here have the required coding standard as compared to the previous ones. Good job!

Your overall code is very neat and readable. Good job!

Generally, your code follows the coding standards pretty well. Keep it up!

I like that you added comments before every method.

Should there be a spacing between the normal bracket and the curly bracket?

Should the else if statement be in the same line as the closing curly bracket?

This line may not be too easy to read. Perhaps split them into multiple lines?

Perhaps extracting this part out would make the code easier to read?

Should the name of the class be singular instead?

Should there be 1 spacing instead of 3 between the curly bracket and "else"?

I like how you make this part of the code readable by refactoring.

Should the specific class that that is used be imported instead of all classes?

I think it would make the code more readable if you replace the 100 with a constant name, maybe "MAX_TASKS" to describe the number?

Also, I think Tasks is a variable - should it be named "tasks" instead? 😃

Hi! I think you can try extracting the code fragments into different methods to increase readability? 😃

Hi! In this case since the indentations are constants, would it be better to name them as finals and in capital letters instead? e.g. "INDENTATION"

((Hi I'm back to do more reviews))

Same thing here - since the wrong warning is a constant string, should it be named WRONG_WARNING instead?

Instead of just welcomeMessage(), I think you can try naming it printWelcomeMessage() so that the methods are verbs and not nouns? 😃

I think it's the same for the other methods below - maybe you can consider naming them as getUserCommand() or something that better describes what the method does?

Hello! I see that you've made the changes from my previous review, and it looks much better now! 👍

I like that the constants are categorised accordingly to their usage like "COMMAND_SOMETHING". 👍

Perhaps you can write more comments to explain how your methods work?

Maybe you can separate the indicators into another method and call it to retrieve it? The main is a little long.

I like how your if-else statements here are very readable!

Perhaps you can write a comment on what do your constants used for?

Perhaps you can use a verb for names representing methods?

Should the method name be written in camelCase?

Perhaps the indentation for wrapped lines can be adjusted to 8 spaces more than the parent line?

Perhaps you can adjust the indentation in the greet() method as well?

Should this be printMessage?

Maybe separate this into another file? (classes Deadline, Event, and Todo)

I like that you included errors

Maybe use camelCase for these methods? (lines 126, 144, 151, 166)

I like that you checked if the command is valid

I think maybe you can extract this part of the code and make it a function to increase readability!

Maybe you could refactor these functions like todo and deadline etc, and make sure that there is less duplicated code by making a function to add task or print the statements after the task is added.

I like that you added a default output for tasks that do not fall under list, done, todo, deadline or event!

getFirstWordOf is a better name

Good use of SLAP principle by separating out the functions

Use curly brackets for one liners as well

Try to make the case where everything runs well more prominent instead of the unusual case (the bye case)

Good use of naming and follows naming convention. It would be good to add a few comments for readability.

Perhaps a good name for your method can be setAsDone() as stipulated in the coding standard

Did you check if your taskCount exceeds your task capacity? And the use ternary operator makes the logic of the code hard to read.

This line is repeated quite a few times. Can you try to refactor into a new function?

Missing spaces here.

Missing a space here.

Better not to put your comment here.

Cool logo.

Instead of 100, you could define a constant like MAX_SIZE as a replacement.

Personally I prefer to align these lines in order to show the full logo completely. And it would be better if you put spaces before and after the "+".

Missing a space here.

It seems that you missed every spaces after the first lines of the methods.

As per coding standards, the class name and "{" should be separated by a space. Eg "...extends Task {...". This also applies to while, for loops, etc.

consider writing variables in camelCase

Consider using camelCase for variable names

Consider using a more descriptive name for Event e

Consider renaming variable "arrOfStr" to be more descriptive

Consider using a more descriptive name instead of "t" for Todo t

Consider importing the needed packages individually

'line" is quite a vague name, maybe you can give a more unique or clearer name to better understand what this variable is used for in the program

There is quite a number of repeated lines here, maybe you can refactor into a function.

The else if and else should be on the same as the } from the previous else

You should only import packages that you need and not import everything from java.util

You may wish to add some short comment for each if, else ifs and else so that others do not need to read what the 'else if' is suppose to check. Also, the else ifs and else should be on the same line as the } from the previous else statement.

You can maybe define a final variable for the value 9 that you used here. Avoid using magic number.

Likewise here, avoid magic number '6'

Imported classes should always be listed explicitly.

you may consider delete brackets inside each case.

better to separate them into a few subclasses.

can change the list to tasks to make it clear

You may consider combining "]" and " "

may consider changing to switch cases

may consider using Task task : listTasks instead of using i

you can consider using the split() method to separate the description and by strings, with " /by " as the separator, this way it'll save a few lines and we need not count the number of characters for substrings

I may be wrong but this method does not seem to be used anywhere in the code! If so, it can be deleted.

Great use of SLAP overall to make the main function extremely readable!

To reduce the number of branches both in the while loop and the addToList method, you can consider just modifying the addToList method to simply taking in the taskList and a Task. Above each addToList, separate methods to create each task object eg. deadline/event can be created to handle the user input. This way, the addToList method will not be handling too many different functions on its own, and we can make use of polymorphism to simplify the method.

it may be good to not have so many chained method calls in one line as it could reduce code readability and efficiency. Perhaps an assignment could be made for userInput.trim().toLowerCase() before the loop!

Everything looks good with regards to coding standard 😃 good job!

Very good use of SLAP to encapsulate the code for each user command. Perhaps the while loop and user input could further be refactored into one method, but overall code quality looks good!

Should this be separated into multiple import statements to show exactly which packages were imported?

I recommend writing each statement of the method on separate lines for increased readability.

Again, it is recommended to not write both statements in one line.

The Duke class contains very long code. Would it be better to separate each class into different java files?

Should these lines be extracted into a method called getTaskIcon()?

I like the idea of storing all the print messages as constants in a separate class!

Should this method be renamed to be a verb? E.g. getSize().

I think it does not have to be so many lines, one is enough

I think this line can be deleted.

This line is too long. Maybe you can do some changes on your addTask method?

I'm not sure if "\u2713" and "\u2718" is very readable? I need to search for this meaning. Or maybe add some comments beside it

I think the else can be put into a new line.

Maybe you can shorten this println by using if else I think?

I think this else can be combine into the switch. Because the default operates the same thing as else, maybe do not need to check valid?

Looks good! I like how the functions for the main code are split into different sub functions

Perhaps a suggestion would be to group the addList and the displayList function into a sort of list class to improve the code quality

LGTM

May I suggest to put the line 13 and 14 into a single line or perhaps to ensure the intentation is around 4 spaces

Looks good! I really like the use of constants in switch statements. It makes things clear

Perhaps for comments of function features, you can add a blank line to separate the function descriptions from the parameters

I like how you verify the task number before running

Like what Arindam mentioned, perhaps try to avoid using shortforms like "dl" to improve readability

same point on avoiding shortforms to improve readability, perhaps consider using a name like getDeadlineDescription()

👍

Perhaps also consider naming this printCompletedTask too 😃

Perhaps consider adding comments before every class declaration such as its inputs, what the object does, what it inherits from, etc 😃

I'm inclined to agree with Arindam

consider using camelCase for variable names like commandLength

For if-else class of statements, perhaps try adding { } for your code that follows 😃

Please avoid deep nesting

I think it is possible to group similar code fragments together via refactoring. I have noticed a few similar areas along with the code.

is it possible to shorten the main method?

is it possible to avoid magic numbers in the array size?

Is it possible to have a better naming convention?

Is it better to define the magic strings with constants?

Is it possible to have verbs in the name of the method? For example, I would think that adding the verb 'print' would make the bye method clearer

Consider the spacing between '=' consistently

Consider the spacing between ','. eg ( blabla(abc, xyz) )

variable is in camelCase and sound 'boolean'. Good!

Plural form name used for a collection of objects. Nice!

The function here clearly shows what they are going to do. Nice!

I would recommend a bracket between each logic statement.

Deep nesting avoided by using &&. Nice!

Great Job! Very neat code.

Could add a comment on the rough output of the string here to increase readability.

Naming for function is abit too long. Perhaps shorten it a little

Good job in extracting out the classes to make the code more readable. Keep it up!

Could perhaps come up with a clearer naming for this function.

Could find a way to refactor this chunk of code under case "done"

Good job in coding practices. Could try to make case statements shorter and more readable by refactoring. Keep up the good work!!

Good coding standards in terms of indentations

Good coding standards with respect to if-else statements

Very readable classes but can be better if class is named as ToDo than ToDos

Could have better naming as compared to just tmp2 and tmp

Excellent readability and appropriate naming for almost all variables.

Overall good naming of all variables and methods

Sufficient and minimal comments overall

Good use of the superclass' toString

Good use of the constants

Good refactoring

A better name for this would be printExit, because exit() sounds like you are terminating the program at this point right away

If these are supposed be be constants, the keyword "final" should be included

if-else should have the following form

if (condition) {

statements;

} else if (condition) {

statements;

} else {

statements;

}

It is better to specify the libraries you have used instead of using a '*'.

Maybe specify what i is

again better to specify what op is

This class looks good

Your code looks good and is pretty easy to read. I like how you have used different methods and not put all of your code in the main method.

Good usage of comments

the at variable name can be changed to something a little more descriptive

Could the method name be changed to doActivity() to reduce the ambiguity from using 'it' instead?

The name listChecker is a little misleading may come across more like a method that does the action of checking rather than a variable. Perhaps you can consider something like itemStatus or activityStatus or something to that effect? The same applies for your Todo, Deadline and Event classes.

Do add white spaces between 'if', the condition and the curly bracket; if statements should have the following form:

if (!commandArgs.isEmpty()) {

I like how you have this!

Not a coding standard violation but... it seems to me that your printAddTodoMsg, printAddDeadlineMsg and printAddEventMsg all do the same thing, I think you can consider putting it all under one method and just changing the method parameters to accept a Task type member variable, then you can use it for all different cases and also shorten your code! 😃

I would suggest leaving whitespace between the method arguments and the opening curly bracket such that:

public void someMethod (String description) {

I noticed the same issue in other files as well, so do try to apply it consistently.

(This rule generally applies for other statements such as if-else, while, for, try, etc.)

I know the variables dlBy and eAt are intuitively for deadline and events respectively, but perhaps you can consider writing it out completely to reduce ambiguity.

Perhaps "100" should be defined as a constant, it seems like a magic number here?

Should there be a space between "if" and "(", and "}" and "else"?

I noticed the same issues in other several places as well (the spacing)

May be you can refactor this part to increase readibility and avoid complicated expressions?

I have noticed the same issue here. May be you can include intermediate steps to avoid complicated expressions?

May be you can input intermediate variables to increase readability?

I have noticed similar mistakes above

Incorrect bracket layout for if-else statements

Comments should have a /** opening on a separate line

Can consider standardising bracket formats:

eg if (....) { } vs if (...)

Can caps LOGO because the string is constant

One of the coding standards is to avoid long methods, maybe you can consider refactoring the code to have a method for analysing inputs, and then from there call the respective functions to carry out the necessary actions

Maybe you can create a method for printing line so that the code appears neater

Maybe the println line can be placed at the start of the while loop instead of within each if-else statement since it will get executed no matter the case

Do remember to add spacing after the class name before adding the braces.

Please ensure consistencies of whether to leave a space after 'catch' before adding the catch conditions.

Perhaps you can comment here on what do '\u2713' and 'u2718' mean respectively?

Perhaps you can delete the blank line here

Any reason why you didn't write '\n' as the constant 'NEW_LINE'?

Perhaps you can use a more descriptive method name?

Perhaps you can fill up line 86 more before wrapping the rest of the code to the next line? Otherwise, I like how you ensure the indentation for the wrapping.

Hello Felix! Perhaps in lines 47 and 86, you could shorten the code line a bit by setting a variable for the conditional statement, then inputting that variable into the System.out.println(). Other than that, the code looks neat!

Hi Felix 😃 Glad to see your reply! Hm, perhaps you could consider using the variable name 'taskTense'? Since the conditional statement here is to implement the appropriate tense for your task(s). Hope this helps!

Hello! Something small I've noticed is that the method Greet() begins with a capital G. Perhaps you could follow the camelCase standard and replace it with greet() instead.

This line is repeated quite a few times in your code. Perhaps you could refactor --> extract method, so as to make your code a bit more concise! 😃

Good use of enums!

Hmm, small issue, but for consistency sake and general neatness of the code, should these empty lines be removed?

For consistency, should there be an empty line here (since you left empty lines between the other cases)?

I like how you made a method to check if your input is valid 👍🏼

overall, good job at following coding conventions! 😃 👍🏼

perhaps, you could add a single-line comment here to explain what this does? 😃

just a suggestion, maybe you could have a method to print your divider or set it as a constant instead 😄

perhaps, you could break up this code into a few other methods? e.g. one method to getInput, another one to printMessage, etc. this will then improve your code's readability (i see lots of deep nesting at the moment) 😃

I see this in all your other case statements as well, but curly braces are not required here! 😃 you can take a look at java's coding conventions here: https://se-education.org/guides/conventions/java/basic.html

perhaps you could put this in a separate method e.g. printWelcomeMessage? same for the bye message 👍🏼 😃

Well done!

No coding standard violation.

Coding quality:

5 is a magic number! Perhaps you could replace 5 with a constant DONE_LENGTH.

More refactoring can be done! The main method is too long. Perhaps you could refactor each case. Example, markTaskAsDone for the code under the done case.

Magic numbers 9 and 4! Perhaps you could make 9 and 4 a constant to DEADLINE_LENGTH and BY_LENGTH

I think endFlag should be a Boolean and it should be named Boolean hasEnded = false instead, to indicate that the loop has exited.

There is a coding standard violation here! if - else loop should be done in this format

if(condition) {

} else if {

}

instead of having else if in another line. 😃

I think your boolean can be name isEmpty instead of b!

Good idea to create this class; improves the code structure and helps with readability in the main class

Variables and classes properly described, easy to understand, good job!

Good idea for refactoring; helps with readability

Not a violation of coding standard but there's an inconsistency between [taskNumber-1] at line 100 and

[taskNumber - 1] at line 104

No violation of coding standard, good job!

No violation of coding standard, PascalCase for Class and camelCase for variable and methods properly followed, good job!

Consider doing this instead "if (stringAfterDone.isEmpty()){ return false; }"

Consider using a more define name for clearer understanding.

There is a space between int i=0 and the semi colon ;

Good use of comment to explain why array have to minus 1.

Consider using public String getTypeOfTask(){ return typeOfTask; }

Good use of naming, clear to reader what this function is about

A spacing helps to see the codes clearer. Eg. "if (txt.equals("list")) {"

Description should be in lower case.

Perhaps a more descriptive name like "welcomeMessage".

Perhaps a more descriptive name to the variable here?

I like the variable name used in this. Every clear and precise.

Perhaps a more descriptive name to the variable here?

Perhaps, you could add comments for what each of the methods does so that reviewers have a brief idea of what the method does just by looking at the comments instead of looking through the codes 😃

Would it be better if you have a fixed FINAL variable for "9", this could come in handy there are changes in the future

Perhaps you can change "validDoneInput" to "isValidDoneInput" to make it sound more like booleans. Anyway, good job 😃

Perhaps you can change the boolean variable to hasSaidBye instead of taskManagerSaysBye

Perhaps you can add some comments before each of the functions for readability

Comments can be added to each of the functions.

e.g.

/**

Prints all tasks in inventory

@param tasks tasks in inventory added by the user

**/

shouldn't task be capitalised since its a class name?

class names should be in PascalCase

I like the way you named your variables

I like how you indented the if statements so that it is not "arrow like"

I feel like this could be a more descriptive name to show what it is indexing.

I like the way you have named these methods, makes easy to tell what they do

else should not be on a new line, and there should be a space before the "{".

else if should not be on a new line, and space before "{".

space before "{"

else if should not be on a new line

rather than using "index" as the variable to track the last element of the array, perhaps using "numberOfInstructions" may be more intuitive to understand.

You may use the partial solution of A-classes to output tick

You may refactor these three lines into a Bye() Method

You may refactor this as a method to print "" or turn "_____" to be a constant

You may add a few comments for people to understand these long methods better.

You may use a switch function when there are more and more comments

I think this is an idiom, do not need to change.

isCompleted

add specifier private

getDate

no need to create another Scanner instance

Extract to a new function

avoid using "should" "must" in the code.

Perhaps this could be plural i.e. 'MAX_TASKS'?

'''suggestion

private static final int MAX_TASKS = 100;

'''

Maybe could add a space before and after the brackets?

'''suggestion

    for (int i = 1; i < taskCount + 1; i = i + 1) {

'''

You can consider using plural form since the variable is for multiple tasks i.e. 'numberOfTasks'.

'''suggestion

protected static int numberOfTasks = 0;

'''

Perhaps 4 could be related to the "/at" string to avoid being a magic number? Could even make "/at" a constant so that you can change it easily.

'''

private static final String PARAM_AT = "/at";

'''

This might not be self-explanatory, so maybe you can add a comment as to what the symbols are?

It's possible this might not follow encapsulation: https://nus-cs2113-ay2021s1.github.io/website/schedule/week3/topics.html#paradigms-oop-objects-encapsulation-of-objects

Maybe make it 'private'? And add a getter for it as well? The access to the variable in 'Duke.java' might need to follow as well.

'''suggestion

private static int taskCount = 0;

public static int getTaskCount() {

return taskCount;

}

'''

It is nice to state who is responding next

Why not refactor the number 100 to specify the meaning of it

What if the user input is not any of these commands or task names? Why not add exception handlers to handle this possible exception haha?

Please specify the meaning of number 100 (MAX_SIZE)

Please specify the meaning of number 40

Nice joke :>

could add some comments to help understand

maybe can put in a single function like printExitMessage()

could declare two final string constants for tick or cross symbol

I think maybe give more information about how "2" comes from which may avoid "magic number" in code

perhaps add some comments for several long functions to help understand

maybe sc is not a good variable name, could change it to a more mnemonic name

What's this semicolumn for?

I like that you add a separate function to create new classes, which makes the code more readable and cleaner. Keep your good job~

I think you did a good job~ You add sperate functions and use understandable function names, it's readable and logical.

I like that you define these static string at the beginning of your code. You used understandable names and made the content more clear.

Perhaps you can change another name instead of allActions?

I like this part that you consider another possible situation to make your code more completed.

unclear what the semicolon is for?

Tasklist can be initialized and assigned a value in the same line Task[] Tasklist = Task[100]

it would be helpful to define taskNumber as one lower if all the applications of the variable needs to be one lower as well

removing the space will help with readability

if else class could use K&R style brackets?

removing space will help with readability (duplicate comment because I need to redo the review)

Consider commenting on the usage of numbers such as 1 and 4 to explain how it affects the indexing.

Functionally correct - but consider using a setter inside the Deadline class itself to avoid abstraction violations.

Functionally correct - but consider using a setter inside the Event class itself to avoid abstraction violations.

Consider using boolean values true/false instead of creating a variable to store those values.

Consider using camelCase naming conventions.

Good systemic use of cases inside the switch statement

Good use of constants here (comment failed to submit on last week tutorial so ignore this).

Also nice use of word wrap.

In general very good following of coding standard for the course: all caps for constants, camelCase for variables and methods etc..

Try to reduce deep nesting here! Break up the if else statements into separate functions or combine certain commands eg int doneTask = Integer.parseInt(words[1]) - 1; is repeated twice.

Minor nitpick with regards to code readability, it's ok if not changed since it is within a class.

Instead of naming your stored String as "by", could name it something more descriptive like deadlineDate or date since when reviewers when Ctrl + F looking the date stored in your class, they might not think to search for "by" since it is a common word used in many classes, functions and comments,

For todo, deadline and event cases in switch, there is duplication of the following code

                System.out.println("Got it. I've added this task:");

                System.out.println(list[inputs].toString());

                System.out.println("Now you have " + (inputs + 1) + " tasks in the list.");

                inputs++;

Try to consolidate this repeated code into a single function eg. void printAddedTask(Task task); for all the System.out functions then call this function for each case. This will help minimise duplication of the code and make your main function more readable.

change "inputs" to "input".

plural form only used for collection of variables/objects

The naming convention for variables is camelCase, so do remember to use the correct naming convention!

Perhaps these variables could be named in another way? You may want to double check the naming conventions for variables and classes/enums as it is quite similar.

Maybe you could add a comment explaining what these variables are meant for!

Perhaps this new line is unnecessary, you may want to remove this so your code looks slightly neater.

i like the comment here that explains what the variables do 😃

I think the naming for variables is camelCase instead of PascalCase!

I like how you used a variable name for MAX_TASK instead of "magic numbers".

Consider having spaces before and after the operator signs like '=' and '>' to make it more readable.

Consider leaving out the "this" word when they are not necessary.

Maybe your main method can be shorten so that it does not exceed 30 LOC?

I like how all your methods have readable names.

I like how your code is not deeply nested so there is no "arrow head" code.

Nice job keeping the length of the lines shorter than 120 characters!

Perhaps you can add in a space before the opening bracket of the if block, so that it follows the code standards.

If I remember correctly, the else if part should be placed on the same line with the closing bracket of the if block above it.

It's very clear what the code is doing, but I would suggest refactoring the code for each case block into their own methods, so that execAddTask doesn't get too long.

Great job organising the code, very clear overview of the program.

Nice use of constants in the switch block!

Hi Jericho. Your code is readable and there are no other serious issues. However, I would like to suggest that you do not need to have the spaces in lines 11 and 13. I think that your code would still be readable even without these spaces. However, if you do feel that these spaces improves readability, do implement them.

Perhaps consider changing the name of the String variable to deadline or dueDate.

Perhaps you could provide a comment regarding what these two Unicode represent. You can define them separately if you want to. This would make it easier to understand what they are.

Hi. Your code looks fine throughout.Just would like to suggest that

you do not have to split lines(61-62) into two seperate lines.They dont seem to exceed the 120 char

limit given to us. Great job nonetheless!

Perhaps consider renaming this variable to date/deadline to be a bit more precise.

Consider leaving an empty line in between the description and parameter for this Javadoc comments

Maybe you can change input.equalsIgnoreCase(LIST_COMMAND) to a function such that it is easier for others to understand what it means?

Perhaps " [E] " can be set as a private variable?

Perhaps you can add a space before date.

Perhaps instead of number of persons, you can say it is number of tasks?

Javadoc of class members can be specified on a single line as follow:

/** Maximum number of tasks that can be held */

Perhaps use /** instead of /* follows the coding standard given.

Perhaps it would be better to put your welcome message and the logo into a method of its own

I think you could format line 102, 103, and 104 to be on the same line since it doesn't exceed the line limit of 120 characters for a single line.

Perhaps a more descriptive variable name for "answer" would be better here?

These nested if-statements were a bit hard to follow, but good job for attempting to handle possible errors!

For a collection of objects, you might want to consider using the plural form. So here instead of calling the array "list" you could call it "tasks" since it is a collection of tasks.

I noticed that for a few cases of if-else statements, you formatted it like:

}

else{

I think it would be better to format it like this: "} else {" with the two ends of the curly braces on the same line as "else".

Hi! I think you could consider making this a final variable!

Hi! For this variable, I think it can be changed into private! Same applies to the other variables in other classes as well.

I like that your main method consists of readable methods which allows readers to guess its functions.

Hi, I think you could consider making this a final variable! Other than that I like how you named your variable name as MAX_TASK.

Hi, I think you can consider making this a private variable! Same goes for the variables in other classes as well!

I like that your main method consists of readable methods, making it easy for readers to understand their functionalities.

Can consider changing this into a switch statement. It's clearer when you have more commands.

Very good coding standard. Keep up with it!

Unnecessary comment

Very neat and clear, good job!

Very good! Your program checks whether user input is valid.

Good coding standard. Your variable names are clear and easy to understand.

Could the variable be changed from an iterator i to a variable name (e.g. spaceIndex). I noticed it in addDeadline() and addEvent() too.

I find it a clever idea to use an enumeration for the command type.

Instead of a while loop, could this be phrased as a for loop instead?

I am not entirely sure whether an empty line counts as a violation, but it might be safer to not leave the line breaks?

Perhaps you can specify whether this is a "public", "protected" or "private" member?

I like your code. Method functions are clear and format is strictly adhered.

Comment should be indented relative to their position in code

Maybe the name can be more specific other than temp1 or temp2?

Maybe you can remove this empty line?

Name should not be a single letter

Comment should be indented relative to their position in code

Do you think that there is an indentation error here? Do consider looking through your code for consistency in code indentation.

Do you think that there is unnecessary amount of spacing here?

Do you think that there is a better way to name this rather than just line?

Can there be a better naming for this variable?

You can consider putting these in a if-else loop?

same issue here, minor indentation error

No indentation for switch statement

Can put this as one of the attribute of class Duke so later static functions you will not need to pass in this array

Can put this into another static (one-line) method to maintain the same level of abstraction

Can extract this part out into a method like printResponseMessage()

Since it's used in todo, ddl and event creation

Try use the toString method of event/ ddl/ toto?

Consider leaving spaces between operators for better readability

Consider the use of javadocs to describe the behavior of your methods.

Consider a different variable name; thing seems a bit too vague.

Consider avoiding the use of AT_LENGTH; it appears to be a "magic number".

Consider a cleaner way to implement commandLength

It might be better to avoid abbreviation?

You can consider following the camelCase format for methods where the first letter is lowercase.

To increase the code readability, you can replace the (... ? ... : ...) statement with an if-else statement. But line 33 still seems quite okay to me.

For the "switch" statement, it would be better if you provide a "default" case also.

To follow the Java coding standard, there should be no indentation for "case" clauses.

Consider using toString for all the print action? As they are repetitive and can be simplified?

Consider importing classes explicitly?

Can consider refactor these lines for done, todo, event, deadline? To make it less clustered

Consider making System.out.println("----------------------------------------"); into a method?

Consider If else loop? To prevent deep nesting

Magic number 100 could be replaced with a constant instead

not the same level of abstraction as the rest of the code in main. perhaps it is better placed in enterGreet()

Good that class name follows upper camel case, also good use of inheritance to prevent repetitive code

There are a lot of implementation details, perhaps it would be even better if you could refactor and abstract most of it into other methods so that other readers have an easier time reading it.

Good that class name follows upper camel case, also good use of inheritance to prevent repetitive code

Should there be spaces between commas to follow the coding standards?

Should there be spaces between "}else" and ")){" to comply with coding standards?

Should Task t have a better variable name?

I like this! Interesting concept of enumerating TaskType here!

I like this! Use of ArrayList so that the list of tasks do not have a fixed capacity!

Perhaps it is good practice to use taskList[i].taskType.equals("T") for string comparison here?

Try to avoid the use of a magic number here. Using a named constant makes the code easier to understand because the name tells us more about the meaning of the number.

Is there a reason to override when the both Task and Todo is similar for this function?

Try to avoid the use of a magic number here. Using a named constant makes the code easier to understand because the name tells us more about the meaning of the number.

Could this be replaced by a traditional for loop? Example :

'''

for (i = 0; i < tasks.length(); i++) {

System.out.println(i+ ". " + tasks[i]);

}

'''

Good work on Level 0.

Good work on Level 1.

Blank lines can be omitted. Otherwise good work on Level 2.

Multiple lines of unnecessary spacings. Would be good if you can omit these blank spaces.

Able to create a new method to replace these 3 lines for organization.

for the if else loop. You can follow this

if (condition) {

statements;

} else if (condition) {

statements;

} else {

statements;

}

Leave a space between else/if and bracket.

And also It is not suggested to use so many if else if.

For example here boolean taskFinished = false;

I think you should use isFinished for boolean variables.

else if(t.description.contains("done")) (should have a space here){

For if-else statements, please follow this

if (condition) {

statements;

} else if (condition) {

statements;

} else {

statements;

}

There should be a space between "if and" (condition) and space between (condition) and "{"

It is not suggested to use so many if else if. I think you can try switch case.

switch (expression) {

case x:

// code block

break;

case y:

// code block

break;

default:

// code block

}

I think you can define a constant variable to avoid magic number like 4,

Would it be better to read if the variables and System.out were separated with a blank line?

It would be more appropriate to name the method as 'printGreeting' instead of 'printGreetings' since the method only contains one greeting.

Perhaps it would be easier to declare a String constant for the line border, so u do not need to type it out every time.

For example, declare a private static final String MESSAGE_BOUNDARY = " _____________________________________"

It feels like the line

else if(command.contains("todo")||command.contains("deadline")||command.contains("event")... seems pretty redundant as well.

Would it be easier to use switch case instead? That would make the logic of duke's response much easier to code, i think.

Would it be possible to use the already existing listSize instead of creating a new method getListSize?

Do consider making a 'decorator' function that can neatly print any output you want to give to the user, with appropriate formatting, divider bars, and newlines.

You can define toString() here as returning the main few things in a 'task printout' - the checkbox and description. Then, you can use super.toString() in the subclasses of Task like so: return "[D] " + super.toString() + " by: " + this.by

Didn't find any coding standard violations, good job!

It's possible for you to use super.toString() in your child classes of Task like Todo, Deadline, etc. such that you don't need to repeat yourself too much.

Do use this.size in line 28 instead of size for consistency.

You can assign the line separator to a string or make a separate line printing function

line can be changed to something like userInput so it is more clear what the string line does

The naming of String by seems a little ambiguous, so it would be good to use a more descriptive variable

It maybe better to set a constant like MAX_TASK_LENGTH = 100 as to avoid the use of magic numbers

You can try adding some more spaces just like the rest of the code

Consider using a switch(input.startsWith("done")) - case block instead of cascading else if statements

Consider making this function return a boolean variable instead of an int

Consider declaring these as static final String in the superclass and naming them with capital letters, eg. MESSAGE_BYE_SIGN

Consider using a more descriptive name like numberOfTasks, and declare it as a static variable under Duke and not in Main

consider SLAPing the codes and using a case-switch block such that each case only contains one function

Perhaps you can break this into methods like printHello, handleInput, to make the code more readable?

I think it is better to name the method with a verb, like markAsDone()

I like how you have the constant MAX_TASKS instead of some magic number!

I like how you define these variables as constants

I like this usage of constants instead of putting the Unicode values

Variable name should be in camelCase

You are accessing a variable named Tasks of class Tasks, the similarity of the name might cause confusion.

To adhere to the code standard, perhaps change the method name to camelCase?

I like the way you implement Java error checking by throw exception.

Perhaps you can add "default" case to adhere with the Code Standard

Refactoring: this is possible to extract this to a new method to improve code readability

There's an unused variable

initialization of variables has to be inside for loop

Switch statements: as per the coding standards, switch statements do no require indentations

boolean naming could be written to sound like a boolean : hasExit()

Hi. The welcome message could be extracted into one method to make the main method shorter and more readable!

Hi, as you can see, the

'int divider = line.indexOf(" ");

String index = line.substring(divider + 1);'

has many repetitions such as the one in line 59,60. These lines share the same purpose, which are to process the 'userinput' to get the command type and content. A such, Extracting the method could be a better way of organizing the code. Hope this can help you!

Hi, as mentioned by the prof during the lecture, 10 is a magic number, and reader like me may not know why the number is set to 10 instead of 100 or else. So you may give the number a meaning by defining it as a constant. Hope this helps!

Hi since the separate line is repeatedly used throughout the main method, you may define it as a static constant outside the main method. This could make your code shorter and more readable!

I like how you organize your code in this switch

Constant used, to be declared in capital letters

Description of return values is mentioned clearly. Good job

Parameter types and return values explained. Easy understanding of the code

Constants used in static, declared in capital letters. Coding standards followed

Constants used for most outputs in the command line, easy readability. Changing these constants would automatically make changes everywhere needed

unnecessary spacing between case

the closing braket should be in line with the 'if'

comment should be put at the top

Interesting use of Enum for Code scalability in future !

I feel importing * is a bad practice as we are uncertain to which file is being imported. To add on, Scanner has already been added prior to importing *. Thus, this line is not needed

Same comment as Level 1

I like that you have handled exception, however, should we be more specific with the error message? So that the user can be more well informed that he/she did not input an integer.

Suppose:

instead of printing "Opps Error", maybe can print "marked index not found/invalid"

Some of the functions have a white space before parentheses, some does not. Should it be more consistent?

You could put the content for each test case in separate functions just like printBye(), so that the code is more uniform and neater

It'll be good to have a constant for 100 so as to avoid magic literals

You could print greet in the showWelcomeScreen() function

Instead of having a chunk of code in main, you could put the code in functions and call the functions from main. Have a look at week 3 lecture on single level of abstraction per function (SLAP).

I think upper case is used for constants and strings are immutable by default. Thus, maybe can change the naming of the variables to fit coding standard.

Maybe can combine the methods together into a single method to make it simpler?

maybe can combine the two ifs and change 1 to if-else

maybe can move all these methods to the Class.java files

Coding Standard: can put comment one line above with the same indentation as the return statement

Coding Standards: Long method, could refactor code

Coding Standards: not wrong, but could use just // for shorter comments

Coding Standards: magic number 100

Perhaps you could consider saving the "u/2713" as a const variable if there is a need to reuse it in the future?

Great use of const to remove ambiguity from magic numbers!

You might consider leaving a space before and after the "=" to improve the readability.

Perhaps you could leave some space in the loop forming stage, for example i < MAX_SIZE; to improve readability.

Instead of using long underline Sting, is it better to declare it once?

I noticed that when you use "+", sometimes you have spacing before or after it, but sometimes don't. Maybe you can adjust the spacing to make it neat.

As for the coding standard, should "else if" statement on the above line with the brace"}"?

This conditional statement seems to be so long. Instead of using the switch statement, maybe you can declare some functions singly to make your codes more concise.

Missing a 'break' statement

"placementCorrect" boolean is not phrased as a boolean. Consider "isPlacementCorrect" instead

Misleading naming convention. "DeadLine" can be interpreted as "Dead Line". Consider "Deadline" instead

Formatting of try-catch statements are inconsistent.

Some are formatted as such:

try {

----endTime = dateWithoutTime.parse(endTimeUnformatted);

} catch (ParseException e) {

----e.printStackTrace();

----return;

}

And the rest are formatted as the highlighted lines.

Good Job! 👍

I like how you abstracted your entire code into functions, allowing for each section to be readable and nameable without need for much comments.

Do we have to use protected here? Would using private suffice?

I like how all command keywords are defined as constants at the start of the file so they can be easily changed when necessary.

I like how your code is abstracted into functions, enhancing readability without need for comments. Good Job! 👍

Just a suggestion, maybe you can try accounting for cases where the taskNo is more than 1 digit.

For example you can split the variable "line" into multiple substrings and then convert the second string to integer as follows:

String[] words = line.split(" ");

int taskNo = Integer.parseInt(words[1]);

Perhaps some of the variable names can be improved further. For example instead of on, can try something like dueOn in your Event class and instead of by, can try completeBy in your Deadline class

The overall structure of the method descriptions is very organized and easy to read. Well done!

Perhaps the underscores can be assigned as a private static variable such that you do not have to manually enter the them in the code multiple times.

I like how you made this additional class to process the user's input.

maybe there should be a comment that explains what arguments string is to improve readability?

I like how all your functions have very clear names

I like how you did this to simplify your print statements

Should this class be renamed as Activity instead of Activities?

Should there be spaces between your methods? I noticed that there is a little inconsistency in several other places too.

Should this space be removed to be consistent with other classes (i.e friendlyBotEvent.java, friendlyBotTask.java)?

I liked how you separated your long lines, making it easily readable!

Good usage of SLAP here. Functions are short and clear and helps make the overall code easy to understand.

The creation of this separate class makes the main Duke file much more clean and readable. Good idea!

There are several unnecessary empty lines here which could be removed.

Perhaps the name function getTaskManagerSaysBye() can be changed to something like hasTaskManagerSaidBye to make it clearer as to what the function is doing.

Maybe you can consider abstracting switch to a new method instead?

Maybe consider replacing 0x2718 and 0x2713 with a named constant to avoid the use of magic numbers?

Is importing this required in this class? If it is not required, would it be better to remove it first?

Maybe you can consider removing these extra empty lines within the if-else statements? Other than that, everything looks good!

Is this an unnecessary line spacing?

Is the return indented too much?

Should this be called directly as taskManager.taskIsChecked(userInputLine) in the main function?

Is this declaration necessary?

Line declaration here when you have a method to printLine

Use the printLine function already implemented

Is isTure intentional? Maybe use a more easily understood name

Perhaps you could consider separating the main method to several other methods so that it is shorter?

Perhaps you may want to consider refactoring the unicode characters as constants for easier reference?

Perhaps you may want to consider refactoring 100 as a constant e.g. MAX_TASKS so as to avoid magic numbers?

Should there be an empty line left in between the description and parameter section? I noticed the same issue in other places too.

Perhaps you could also add a punctuation behind each parameter description.

Should there be a space to separate variables and method?

Any reason why you used if-else statements instead of switch statement?

Good job on your indentation. Code is organised

I like this part as this function compiles all possible command the user can input, and then give these tasks to the respective functions.

This is a good practice of enum!

I like this function as we are using the line everywhere of the iP, extract this as a function is a wise choice. However, maybe 100 '-' is too long?

Perhaps NoOfIncompleteTasks would be a better name?

The loop body should be wrapped by curly braces, even though there is only one line of code in the body

I noticed that the same chunk of code (e.g. lines 48-51, 55-58 etc) is repeated for every addition of a new item. Perhaps you could consider modularising this into one addTask function, in order to avoid unnecessary repetition?

Do remember to follow Egyptian style for the curly braces. 😃

Could you explain the code in your constructor? Because (as far as I know) a constructor should only be used to create an object, assign values to attributes and call other class methods (if needed). I think you could consider abstracting the rest of the code into a separate printTask function.

I think this is a good idea! Making a separate class for constants. Makes your code neater

I like how you have put them in different functions for different types of inputs by refactoring the code. Makes the code more readable.

Good using of white spaces, makes the code very readable !

I think overall, good use of the Egyptian style for the brackets. Maybe naming for the methods can be improved. Code is readable too!

perhaps can consider adding empty lines in between to improve readability

perhaps can just add the code in the two methods directly into echoCommand, i didn't see the point of creating 2 other methods

same as echoCommand, perhaps can consider commenting in the code to separate the 2 parts. since reader would have to go and look for the 2 methods, then be able to understand. decreased readability of code

perhaps can put the while loop that handles commands into a function instead of main

Methods should be separated by one blank line

Else statements should be inline with the bracket preceding it. ie. "} else {"

some methods have a space between method name and the open brackets while others doesnt. ie. "Todo {" versus "Todo{". A solution would be to use Ctrl + Alt + L in Intellij to reformat the code easily.

There should be spaces between operators. ie. "indexDivider + 3" instead of "indexDivider+3"

Hi, operators should be surrounded by a space character, just like line 6.

Good use of named constant 'MAX_LIST_SIZE'!

avoid magic number like 100

variable names should be in camel case ie taskNumber

You may want to edit the format of the if-else statement such that it follows this form:

if (condition) {

statements;

} else if (condition) {

statements;

} else {

statements;

}

Just like what you did the the sections above.

The rest of the code looks fine. Good Job!

Try to be consistent in the while loop and if loop, as some got space before the open bracket, some no

You may consider using switch cases. It will looks neater.

Your while loop looks like missing a space. Maybe you can go to the coding standard and compare it .

Please use correct spaces "=="

Should the "+" sign at this line or at the next line?

Should the "+" sign at this line?

There are multiple repeated lines in string "__________" , you might consider to use a variable to reduce?

Instead of importing the enttire package, it would be better to just import the class you need

Instead of having so many system.out.println commands, it would be better to refactor these into a method itself

Hi,

instead of using 100 which is a magic number, you can give meaning by defining it as a constant.

Could consider extracting this segment into a single method to inform user of success operation

Good and clean use of the switch statement

Consider adding a space after "while"

Maybe you can use the SLAP technique here and refactor this into a new function

can use a better variable name to call the object

Maybe you could add a space after the closing curly brackets } and then type "else"

I like this way you inherit and simplify the task.

It's clear to separate the tasks with a comment.

It might be more clear if you put this function before main

According to the coding standard, shouldn't due_date be dueDate?

Perhaps the function name could change to shouldQuit or isDone? When the input is "bye", the program should quit.

Perhaps can replace desc with description? Took me a second to realise it's description.

i think the naming of this variable is a bit confusing. It is used to check mainly the first 2 word , and not just for "done". Also since it only checks the first two word i think do not really need to create an array of the whole sentence.

I think it would be clearer to create another method for printing the text and then call from main instead of putting it in the constructor.

Coding standard violation: else if should be in the same line as the end bracket.

Perhaps you could store the boolean in a variable and return it at the end of the method, instead of returning in the middle of the switch statement.

This section would be easier to read if the commented out code was placed at the end instead of in the middle.

Instead of naming the list "list", you can consider using a more descriptive name like "taskList".

due_date should be written as dueDate as per the coding standards

Perhaps you should use isFinished instead of taskFinished

Should not have multiple return lines in a single method

Same!

Instead of using arrays and counting the number of tasks. You could also use arraylists from java docs to access more functionalities

You dont have to use 4 back to back System.out.println. The use of /n actually creates a new print on the next line.

Could there be an inconsistency with the spacing?

I think there could be a similar

Hi, just thought that since this is a method, it should start with small letters instead of capital letters, small issues

code is nice & readable, easy to uds 😃

i agree with benardo, maybe spacing will help!

i like the comments! makes the code more readable

Perhaps, you could use less abbreviation for the variable names to make it more straightforward i.e. deadlineDate.

I think using the variable name such as noCommand can be a little bit confusing. I would suggest using variable names that are less confusing. noCommand can mean 2 things, 1) no command and 2) number of commands.

Perhaps you can opt for more straight forward variable name for the obtainDLDesc, such as obtainDeadlineDescription

The code seems a bit repetitive here. It would be great if you modify this part to be less repetitive. The lines here also seem to be a bit long.

I like how you changed the functions to private

I like how you are able to reuse super.toString() here

using switch case will be better

instead of putting so many print lines under one condition, extract them and name them according to their commands

Your switch case should have proper indentation

I think if-else standards need to be in this form:

if (...) {

...

} else if(...) {

...

}

I think these numbers might be viewed as magic numbers

I really like your coding styles. It is super clear and easy to understand and to read as well. They weren't any magic numbers and the refactoring is awesome!

I like the use of extraction to determine the task type, display list and task done function. It makes the code more concise and easy to read.

Good job, your code looks clean and good.

Great job on extracting your code into different readable functions with clear function heading.

And also a fantastic job on error handling.

I think it's better to put the main method at the bottom and declare other methods at first, which is more comfortable to read that people don't need to scroll down the code frequently to figure out where you declare and how you implement it.

This while loop can be changed to a separate method outside the main method, which will make the main method much shorter

I think separating this while loop into mutiple methods such as processTodo, processDeadline, would be more elegant and easy for people to read

Might be good to specify in a way that these are HTML codes for ticks and crosses,, i would consider it as borderline magic number?

All the else and if else should be right after the closing bracket of the previous condition.

Good coding practice!

Preferably change line when using curled brackets to organize them better.

would be better to not use '*', replace with the 'scanner...

Very easy to understand, good to go

I think maybe it would be better if you use case statements instead of keep on using (else if) for this type of tasks

Maybe you could put all these codes into deadline class to make your main neater

Should you avoid using this if it is unnecessary, aka when it is clear that the variable is referring to the class element? It will also be good to keep it consistent as you used this.isComplete in checkIsComplete() but only isComplete in getCompletionStatusIcon. Other than this, well done!

Well done 😃 I think it is good that you have abstracted the individual commands into their own function instead of putting them all in this function.

It is good practice to use Constants instead of Magic Numbers

Good use of Constants

Redundant overide that doesn't change super class method. Consider removing the override to shorten the code.

Nice spilt method to split string into 2 seperated by '/'

Could instead use "eventDate"

Magic value here, perhaps use a string variable to hold the tick and cross

Great implementation! I think you could keep indentation within the previous level.

Well done! You could refactor to check and break out of errors, and leave the main logic in the main indentation.

for this part of condition statements, it has more than 3 levels of indentation. So may create some subfunctions to reduce the number of indentations.

some lines here are a little bit longer and with many functions in it. May break them into parts. Also can leave some comments to explain them

Nice code, very neat.

nice code, very neat.

Any reason why you used if-elseif-else instead of switch here?

I like how readable this section is.

Hi! I think you may have missed out the explicit "//fallthrough" comment for case statements that do not have a break statements.

I like how you applied SLAP principles in your code!

Perhaps you can refractor such constant statements

Perhaps you can refractor the number 100 to show the meaning of it since its a constant?

i like how you used switch / case statements here 👍

nice job, i like how clean the code is 👍

The line "Got it..." is repeated a few times. Can consider restructure your code to reduce redundancy.

However, the use of space is not consistent here, some methods have space before curly braces while some do not, like this one

Like the use of constants for each command

Suggestion would to be have a method for the addition of each task type rather than to handle it in case statements

Good job using capital letters to assign constants.

Well done! Good readability and abstraction for the code.

Your code looks clean but I would suggest putting spaces like "} else {" to improve readability. but otherwise well done! 😃

Perhaps a more descriptive naming for this would be better? like userInput etc. And I noticed some of your comments are explained in the method name already, would it be more appropriate to remove them in that case? But otherwise it looks it looks good to me! 😃

'else if' should start on the same line as the end brackets.

good code quality!

I like that you write multiple functions representing different reactions the system is supposed to have when receiving different inputs, which makes the code very readable, but I think you may consider adding some comments before some code, especially in long functions, so that it is easier for you to modify it afterwards, also easier for reviewers.

I like how you created a lot of methods to represent different processing of different inputs, which makes the code very readable, but it might be better to put all these functions as methods of the Task class?

would be good to refactor the printing of duke logo into a method

Good job on the code!! Nothing much that needs changing 😃

Overall, the code is very readable. However, the main method is quite long. It could be better to apply SLAP coding principles and make multiple methods to handle the many variables in main().

In addition, some of the method names are not very clear. For example, what is a traditional task? Perhaps a few lines of comments to explain such names, or finding more apt names could be better.

Overall, the code is readable and mostly follows the coding standards.

However, the method names, while still somewhat reflecting the methods' purposes, deviate slightly from the coding standards prescribed. For example, the method farewell, is a single noun. Perhaps it could be better renamed printFarewellMessage. Another example is listPrinter, which is not a verb, but a noun. Perhaps it could be better named printList.

Still, there are many other instances where the naming is properly done, such as in markDone and addedTask, and so these slight errors may be due to carelessness

Hi Sean, Good Job on your code I found no violation to the coding standards in your code. Just one comment i don't think you have to expliciltly import your classes because they are all kept in the same file other than that everything seems great.

Good Job on your code for the individual Project. A small comment I noticed a spacing between the parenthesis and the starting curly brace for some of your methods but other than that I did not find any issues.

Perhaps you can put tasks[taskCount++] instead to reduce the code by one line

This part is quite heavily nested and lengthy, consider extracting part of it as a separate method. Other than that, pretty clean code!

You may consider doing a SLAP for the codes within the while loop into different methods to improve readability.

iP review comments dashboard

1. LAM ..ROLD @jerroldlam (32 comments)

2. ONG .. ZHI @OngDeZhi (27 comments)

3. LI B..ZHAO @TomLBZ (24 comments)

4. NEIL..RJEE @neilbaner (19 comments)

5. DANI..OONG @daniellimws (17 comments)

6. JOHA..RDJI @johanesrafael (15 comments)

7. CHAN.. HAO @ChanJianHao (15 comments)

8. TAN ..ELIA @AmeliaTYR (15 comments)

9. YEOW..NDON @brandonywl (14 comments)

10. EYO ..CHIN @EyoWeiChin (14 comments)

11. MATT..GANI @matthewgani (14 comments)

12. ANDR.. HUA @AndreWongZH (13 comments)

13. CHAN..LING @chocomango (13 comments)

14. TRA ..HONG @Promethees (13 comments)

15. YEN ..FANG @pinfang (13 comments)

16. ZHAO..IANG @e0426051 (13 comments)

17. NG W..IONG @weisiong24 (12 comments)

18. TANG..TING @t170815518 (12 comments)

19. TAN ..HONG @TanJunHong (12 comments)

20. TANG.. YOU @Zhi-You (12 comments)

21. VISH..RORA @wish2023 (12 comments)

22. ZHAN..IAYI @gua-guargia (12 comments)

23. VISH..NJAN @sevenseasofbri (12 comments)

24. MUHA..WALA @MuhammadHoze (11 comments)

25. HUYN..RANG @alwaysnacy (11 comments)

26. KING..IANG @kstonekuan (11 comments)

27. ZHAN..ILIN @Zhangyilin0203 (11 comments)

28. ONG .. BIN @n3wsoldier (11 comments)

29. LIU ..FENG @LIU-YiFeng-1 (11 comments)

30. KWEK.. HAO @Speedweener (11 comments)

31. WAMI..ALIK @wamikamalik (11 comments)

32. ARIN..ATHI @arindamshivatrip (11 comments)

33. ZOU ..FENG @teachyourselfcoding (10 comments)

34. LIU ..ORAN @yuqiaoluolong (10 comments)

35. LOOI.. WEN @kaiwen98 (10 comments)

36. CHEN.. JUN @iamchenjiajun (10 comments)

37. TEOH..IANG @zhixiangteoh (10 comments)

38. LYU ..AWEN @JiawenLyu (10 comments)

39. DANZ..HERN @dojh111 (10 comments)

40. JOSE..AHAM @josephhhhhhhhh (10 comments)

41. NEO ..JOEL @yAOwzers (10 comments)

42. ZEON..EIYI @CFZeon (10 comments)

43. JOHN..IANG @JohnNub (10 comments)

44. HUBE..AWAN @adhy-p (10 comments)

45. CHUA..ANDA @amanda-chua (10 comments)

46. YE Y..TONG @yeyutong811 (10 comments)

47. CHON.. LIN @czlin98 (10 comments)

48. NGUY..KHOA @tienkhoa16 (10 comments)

49. ZHAN..MING @snowbanana12345 (9 comments)

50. GUO ..O AI @GuoAi (9 comments)

51. DUAN..HANG @Artemis-Hunt (9 comments)

52. ADI ..NATA @adinata15 (9 comments)

53. WANG..YING @WYing333 (9 comments)

54. THEO..INTO @theopin (9 comments)

55. KHEN..IANG @Khenus (9 comments)

56. TOH ..NDON @brandontoh (9 comments)

57. VO Q..HUNG @hungvo0603 (9 comments)

58. NUR ..SANI @amalinasani (9 comments)

59. LIM ..G LE @limgl1998 (9 comments)

60. KWA ..MOND @DesmondKJL (9 comments)

61. YEE ..NSON @Johnson-Yee (9 comments)

62. ANG ..YANG @acyang97 (8 comments)

63. YEAP.. LIK @yeapcl (8 comments)

64. HENG..YUEN @HengFuYuen (8 comments)

65. R RA..MANA @R-Ramana (8 comments)

66. LIEW..SIEW @LiewWS (8 comments)

67. WANG..NNAN @AnnanWangDaniel (8 comments)

68. KER ..TIAN @kerct (8 comments)

69. ZHU ..ZEYU @Zhu-Ze-Yu (8 comments)

70. LU Z..ZIYI @luziyi9898 (8 comments)

71. HO M.. JUN @homingjun (8 comments)

72. KENN.. EER @KennethEer (8 comments)

73. SEAN..IANG @Aseanseen (8 comments)

74. DANI..-JIE @daniellimzj (8 comments)

75. TOLE..ALES @abnermtj (8 comments)

76. LEE ..YUAN @lhydl (8 comments)

77. TEY ..RONG @xingrong123 (8 comments)

78. SEAH..HLOE @chloesyy (8 comments)

79. ZHAN..NJIA @jessicazhang617 (8 comments)

1. LAM ..ROLD `@jerroldlam` (32 comments)

2. ONG .. ZHI `@OngDeZhi` (27 comments)

3. LI B..ZHAO `@TomLBZ` (24 comments)

4. NEIL..RJEE `@neilbaner` (19 comments)

5. DANI..OONG `@daniellimws` (17 comments)

6. JOHA..RDJI `@johanesrafael` (15 comments)

7. CHAN.. HAO `@ChanJianHao` (15 comments)

8. TAN ..ELIA `@AmeliaTYR` (15 comments)

9. YEOW..NDON `@brandonywl` (14 comments)

10. EYO ..CHIN `@EyoWeiChin` (14 comments)

11. MATT..GANI `@matthewgani` (14 comments)

12. ANDR.. HUA `@AndreWongZH` (13 comments)

13. CHAN..LING `@chocomango` (13 comments)

14. TRA ..HONG `@Promethees` (13 comments)

15. YEN ..FANG `@pinfang` (13 comments)

16. ZHAO..IANG `@e0426051` (13 comments)

17. NG W..IONG `@weisiong24` (12 comments)

18. TANG..TING `@t170815518` (12 comments)

19. TAN ..HONG `@TanJunHong` (12 comments)

20. TANG.. YOU `@Zhi-You` (12 comments)

21. VISH..RORA `@wish2023` (12 comments)

22. ZHAN..IAYI `@gua-guargia` (12 comments)

23. VISH..NJAN `@sevenseasofbri` (12 comments)

24. MUHA..WALA `@MuhammadHoze` (11 comments)

25. HUYN..RANG `@alwaysnacy` (11 comments)

26. KING..IANG `@kstonekuan` (11 comments)

27. ZHAN..ILIN `@Zhangyilin0203` (11 comments)

28. ONG .. BIN `@n3wsoldier` (11 comments)

29. LIU ..FENG `@LIU-YiFeng-1` (11 comments)

30. KWEK.. HAO `@Speedweener` (11 comments)

31. WAMI..ALIK `@wamikamalik` (11 comments)

32. ARIN..ATHI `@arindamshivatrip` (11 comments)

33. ZOU ..FENG `@teachyourselfcoding` (10 comments)

34. LIU ..ORAN `@yuqiaoluolong` (10 comments)

35. LOOI.. WEN `@kaiwen98` (10 comments)

36. CHEN.. JUN `@iamchenjiajun` (10 comments)

37. TEOH..IANG `@zhixiangteoh` (10 comments)

38. LYU ..AWEN `@JiawenLyu` (10 comments)

39. DANZ..HERN `@dojh111` (10 comments)

40. JOSE..AHAM `@josephhhhhhhhh` (10 comments)

41. NEO ..JOEL `@yAOwzers` (10 comments)

42. ZEON..EIYI `@CFZeon` (10 comments)

43. JOHN..IANG `@JohnNub` (10 comments)

44. HUBE..AWAN `@adhy-p` (10 comments)

45. CHUA..ANDA `@amanda-chua` (10 comments)

46. YE Y..TONG `@yeyutong811` (10 comments)

47. CHON.. LIN `@czlin98` (10 comments)

48. NGUY..KHOA `@tienkhoa16` (10 comments)

49. ZHAN..MING `@snowbanana12345` (9 comments)

50. GUO ..O AI `@GuoAi` (9 comments)

51. DUAN..HANG `@Artemis-Hunt` (9 comments)

52. ADI ..NATA `@adinata15` (9 comments)

53. WANG..YING `@WYing333` (9 comments)

54. THEO..INTO `@theopin` (9 comments)

55. KHEN..IANG `@Khenus` (9 comments)

56. TOH ..NDON `@brandontoh` (9 comments)

57. VO Q..HUNG `@hungvo0603` (9 comments)

58. NUR ..SANI `@amalinasani` (9 comments)

59. LIM ..G LE `@limgl1998` (9 comments)

60. KWA ..MOND `@DesmondKJL` (9 comments)

61. YEE ..NSON `@Johnson-Yee` (9 comments)

62. ANG ..YANG `@acyang97` (8 comments)

63. YEAP.. LIK `@yeapcl` (8 comments)

64. HENG..YUEN `@HengFuYuen` (8 comments)

65. R RA..MANA `@R-Ramana` (8 comments)

66. LIEW..SIEW `@LiewWS` (8 comments)

67. WANG..NNAN `@AnnanWangDaniel` (8 comments)

68. KER ..TIAN `@kerct` (8 comments)

69. ZHU ..ZEYU `@Zhu-Ze-Yu` (8 comments)

70. LU Z..ZIYI `@luziyi9898` (8 comments)

71. HO M.. JUN `@homingjun` (8 comments)

72. KENN.. EER `@KennethEer` (8 comments)

73. SEAN..IANG `@Aseanseen` (8 comments)

74. DANI..-JIE `@daniellimzj` (8 comments)

75. TOLE..ALES `@abnermtj` (8 comments)

76. LEE ..YUAN `@lhydl` (8 comments)

77. TEY ..RONG `@xingrong123` (8 comments)

78. SEAH..HLOE `@chloesyy` (8 comments)

79. ZHAN..NJIA `@jessicazhang617` (8 comments)

80. FANG.. HUI `@hui444` (8 comments)