Sharing iP code quality feedback [for @howenc] - Round 2 #4
Description
@howenc We did an automated analysis of your code to detect potential areas to improve the code quality. We are sharing the results below, so that you can avoid similar problems in your tP code (which will be graded more strictly for code quality).
IMPORTANT: Note that the script looked for just a few easy-to-detect problems only, and at-most three example are given i.e., there can be other areas/places to improve.
Aspect: Tab Usage
No easy-to-detect issues 👍
Aspect: Naming boolean variables/methods
Example from src/main/java/task/ListOfTask.java lines 91-91:
LocalDateTime endDayDateTime, boolean print) throws DukeSaveException {Suggestion: Follow the given naming convention for boolean variables/methods (e.g., use a boolean-sounding prefix).You may ignore the above if you think the name already follows the convention (the script can report false positives in some cases)
Aspect: Brace Style
No easy-to-detect issues 👍
Aspect: Package Name Style
No easy-to-detect issues 👍
Aspect: Class Name Style
No easy-to-detect issues 👍
Aspect: Dead Code
No easy-to-detect issues 👍
Aspect: Method Length
Example from src/main/java/storage/Storage.java lines 76-127:
private static String load(ListOfTask taskList, int startLine, String errorCarryForward) {
assert(startLine > 0);
assert(errorCarryForward != null);
File holderDirectory = new File("./duke");
File saveDirectory = new File("./duke/data");
File saveData = new File("./duke/data/duke.txt");
String error = null;
try {
holderDirectory.mkdir();
saveDirectory.mkdir();
saveData.createNewFile();
Scanner readData = new Scanner(saveData);
// Continue from startLine
for (int i = 0; i < startLine - 1; i++) {
if (readData.hasNextLine()) {
readData.nextLine();
}
}
// Begin loading
while (readData.hasNextLine()) {
String command = readData.nextLine();
error = command;
// If next line is empty, it is the end of file
if (command.equals("\n")) {
break;
}
// Loads each line in the save file into the taskList
Parser cmd = new Parser(command);
Commands action = cmd.parse();
action.execute(taskList, false);
startLine++;
}
readData.close();
} catch (DukeException e) {
startLine++;
// Carries the error to be shown at the end of the load
return load(taskList, startLine, errorCarryForward
+ "line " + startLine + " corrupted: " + e.getMessage() + "\n" + error + "\n");
} catch (IOException f) {
return "You do not have access to create a save file";
}
return errorCarryForward;
}Example from src/test/java/command/CommandsTest.java lines 69-109:
public void commandsExecuteEvent() {
String[] cmd = {"event CS2103T A-JUnit /from 18-09-2023 0000 /to 18-09-2024 0000",
"event CS2103T A-JUnit /from 18-09-2023 0000 /to 18-09-2023 0000",
"event CS2103T A-JUnit /from 18-09-2023 0000 /to 18-09-2024 9999",
"event CS2103T A-JUnit /from 18-09-2023 0000 /to 18-09-2024 ",
"event CS2103T A-JUnit /from 18-09-2023 /to 18-09-2024 0000",
"event CS2103T A-JUnit /from 18-09-2023 0000 /to",
"event CS2103T A-JUnit /from 18-09-2023 0000 /t 18-09-2024 0000",
"event CS2103T A-JUnit /from 18-09-2023 0000 / 18-09-2024 0000",
"event CS2103T A-JUnit /from 18-09-2023 0000 18-09-2024 0000",
"event CS2103T A-JUnit /from 18-09-2023 0000",
"event CS2103T A-JUnit /from 18-09-2023 000",
"event CS2103T A-JUnit /from 18-09-2023 9999",
"event CS2103T A-JUnit /from 18-09 1010",
"event CS2103T A-JUnit /from",
"event CS2103T A-JUnit /fro",
"event CS2103T A-JUnit /from /to ",
"event CS2103T A-JUnit"};
String cmd2 = "CS2103T A-JUnit";
String cmd3 = "added: [E][ ] ";
String cmd4 = " (from: 18-09-2023 0000 to: 18-09-2024 0000)";
for (String str : cmd) {
Parser cm = new Parser(str);
final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
System.setOut(new PrintStream(outContent));
try {
Commands c = cm.parse();
assertEquals(cmd3 + cmd2 + cmd4, c.execute(new task.ListOfTask()));
} catch (DukeFromEarlierThanToException e) {
assertEquals("From must be earlier than To", e.getMessage());
} catch (DukeDateTimeParseException e) {
assertEquals("The format for dates&time is 'dd-MM-yyyy hhmm'", e.getMessage());
} catch (DukeNullPointerException e) {
assertEquals("The format for the command is: "
+ "event task /from startDayDateTime /to endDayDateTime",
e.getMessage());
} catch (DukeException e) {
assertEquals("Please add the task name", e.getMessage());
}
}
}Example from src/test/java/command/CommandsTest.java lines 112-152:
public void commandsExecuteMark() {
String[] cmd = {"todo CS2103T A-JUnit",
"deadline CS2103T A-JUnit /by 18-09-2023 0000",
"event CS2103T A-JUnit /from 18-09-2023 0000 /to 18-09-2024 0000"};
String[] cmdi = {"[T][X] CS2103T A-JUnit",
"[D][X] CS2103T A-JUnit (by: 18-09-2023 0000)",
"[E][X] CS2103T A-JUnit (from: 18-09-2023 0000 to: 18-09-2024 0000)"};
ListOfTask taskList = new ListOfTask();
for (String str : cmd) {
Parser cm = new Parser(str);
try {
Commands c = cm.parse();
c.execute(taskList);
} catch (DukeException e) {
System.out.println("check the test cases again");
}
}
int[] indexes = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
for (int i = 0; i < indexes.length; i++) {
try {
final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
System.setOut(new PrintStream(outContent));
String str = taskList.markOrUnmark(indexes[i] + 1, true, true);
assertEquals(cmdi[i], str);
} catch (DukeNumberFormatException e) {
assertEquals("Place a number after the command", e.getMessage());
} catch (DukeException e) {
assertEquals("Please select from index 1 to " + taskList.size(), e.getMessage());
}
}
final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
System.setOut(new PrintStream(outContent));
String s = "mark a";
Parser p = new Parser(s);
try {
Commands c = p.parse();
} catch (DukeException e) {
assertEquals("Place a number after the command", e.getMessage());
}
}Suggestion: Consider applying SLAP (and other abstraction mechanisms) to shorten methods e.g., extract some code blocks into separate methods. You may ignore this suggestion if you think a longer method is justified in a particular case.
Aspect: Class size
No easy-to-detect issues 👍
Aspect: Header Comments
Example from src/main/java/task/ListOfTask.java lines 161-169:
/**
* Mark or unmark a task as done.
*
* @param index The index of the task based on the current task list.
* @param markOrUnmark True to mark, false to unmark.
* @param print True to print messages, false to not print messages.
* @return Returns an empty string if print is false and a message if print is true.
* @throws DukeException If the number is outside the range of indexes in the list.
*/Example from src/main/java/task/ListOfTask.java lines 190-196:
/**
* Delete a task from the task list.
*
* @param index The index of the task based on the current task list.
* @return Returns a message string if print is true and null if false.
* @throws DukeException If the number is outside the range of indexes in the list.
*/Example from src/main/java/task/Task.java lines 29-31:
/**
* Mark the task as done.
*/Suggestion: Ensure method/class header comments follow the format specified in the coding standard, in particular, the phrasing of the overview statement.
Aspect: Recent Git Commit Message
possible problems in commit f2fb1b8:
Creating a UserGuide
- Not in imperative mood (?)
possible problems in commit 2dffb9d:
Refactoring and Code Quality
- Not in imperative mood (?)
Suggestion: Follow the given conventions for Git commit messages for future commits (do not modify past commit messages as doing so will change the commit timestamp that we used to detect your commit timings).
Aspect: Binary files in repo
No easy-to-detect issues 👍
❗ You are not required to (but you are welcome to) fix the above problems in your iP, unless you have been separately asked to resubmit the iP due to code quality issues.
ℹ️ The bot account used to post this issue is un-manned. Do not reply to this post (as those replies will not be read). Instead, contact [email protected] if you want to follow up on this post.