Skip to content

[shawnpong] iP#178

Open
shawnpong wants to merge 42 commits into
nus-cs2113-AY2324S2:masterfrom
shawnpong:master
Open

[shawnpong] iP#178
shawnpong wants to merge 42 commits into
nus-cs2113-AY2324S2:masterfrom
shawnpong:master

Conversation

@shawnpong

Copy link
Copy Markdown

No description provided.

damithc and others added 11 commits January 7, 2024 18:33

@Joshuahoky Joshuahoky left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider refactoring main in Floda.java into different methods to make the code easier to read and understand. Otherwise, code follows Java coding standard well and is of high quality

this.by = by;
}

@Override

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great use of Override annotation to signal method overriding

Comment thread src/main/java/Floda.java Outdated
} else {
System.out.println("Invalid task number! Please check with 'list'.");
}
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid arrowhead style code

Comment thread src/main/java/Floda.java
Comment thread src/main/java/Floda.java Outdated

public class Floda {
public static void main(String[] args) {
String name = "Floda";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Redundant code as it can be combined with the next line
eg. System.out.println("Hello! I'm Floda");

@lordgareth10 lordgareth10 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks okay to me, the coding standards were mostly adhered to in general, however some parts which I commented on could be refactored to improve logical flow and readability.

Comment thread src/main/java/Task.java

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could consider refactoring to use separate methods to handle individual command types. For example, if the command is "event" you could use a handleEventInput() method to parse the string and add the object to the list. This will improve readability considerably.

Comment thread src/main/java/Floda.java
@@ -0,0 +1,83 @@
import java.util.Scanner;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like how you specified java.util.Scanner instead of java.util.* , this complies with coding standards, well done !

Comment thread src/main/java/Floda.java Outdated

public class Floda {
public static void main(String[] args) {
String name = "Floda";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should this be a constant? And if it was then perhaps you could have used (public static final string) and capitalised the variable name (NAME) to follow the coding standards.

Comment thread src/main/java/Floda.java Outdated
Comment on lines +12 to +27
while (!"bye".equals((line = scanner.nextLine()))) {
if ("list".equals(line)) {
System.out.println("List so far: ");
for (int i = 0; i < taskCounter; i++) {
System.out.println((i + 1) + "." + list[i]);
}
} else if (line.startsWith("mark")) {
Scanner taskScanner = new Scanner(line);
taskScanner.next();
if (taskScanner.hasNextInt()) {
int taskNumber = taskScanner.nextInt() - 1;
if (taskNumber >= 0 && taskNumber < taskCounter) {
list[taskNumber].setDone(true);
System.out.println("I have marked this task as done:\n" + list[taskNumber]);
} else {
System.out.println("Invalid task number! Please check with 'list'.");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perhaps you could refactor the code into smaller methods that handle the logic without having to have so many levels of nesting, this would make the code more readable.

Comment thread src/main/java/Task.java
this.description = description;
}

public boolean isDone() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like how the boolean method isDone is namd to sound like a boolean!

Comment thread src/main/java/Floda.java Outdated
Comment on lines +11 to +12
System.out.println("I can keep track of a to-do list for you! Just type what you want to add to the list.");
while (!"bye".equals((line = scanner.nextLine()))) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would improve readability if you directly assigned String line = scanner.nextLine() in the previous line instead of assigning it inside the conditional for the while loop. Then you could read in a new line of input at the end of every if else block.

@nichyjt nichyjt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi Shawn,

Your code is generally well structured. Good job!

There are some repeated mistakes in the codebase (such as magic literals), so do take note.

There are some other specific comments below. Try and fix them before the next tutorial!

For your consideration: There is a way to autoformat your code automatically every time you Ctrl-S on intellij which will save you much pain. :)

Comment thread src/main/java/Floda.java Outdated
Comment on lines +11 to +12
private final static String NAME = "Floda";
private static final String FILE_PATH = "./data/tasks.txt";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure why the NAME constant left out the static keyword?

Comment thread src/main/java/Floda.java Outdated
Comment thread src/main/java/Floda.java Outdated
Comment thread src/main/java/Floda.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants