Sharing iP code quality feedback [for @howenc] #3
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, to help you improve the iP code further.
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/parser/Parser.java lines 44-134:
public Commands parse() throws DukeException {
Commands.CommandEnum mainCmd = this.mainCommand();
switch (mainCmd) {
case BYE: case LIST: case UNDO:
return Commands.of(mainCmd);
case TODO: case FIND:
if (this.secondString() == null) {
throw new DukeException("Please add the task name");
} else {
return Commands.of(mainCmd, this.secondString());
}
case BY: case FROM: case TO:
try {
String restOfCommand = this.secondString().trim();
LocalDateTime dateTime = LocalDateTime.parse(restOfCommand, Duke.FORMAT);
return Commands.of(mainCmd, dateTime);
} catch (DateTimeParseException | NullPointerException e) {
throw new DukeDateTimeParseException("The format for dates&time is 'dd-MM-yyyy hhmm'");
}
case MARK: case UNMARK: case DELETE:
try {
int index = Integer.parseInt(this.secondString());
return Commands.of(mainCmd, index);
} catch (NumberFormatException | NullPointerException e) {
throw new DukeNumberFormatException("Place a number after the command");
}
case DEADLINE:
try {
String task = this.commandPhaseParse();
String command2 = this.phaseTwo();
if (task == null) {
// No task name
throw new DukeException("Please add the task name");
}
Parser phaseTwo = new Parser(command2);
Commands c = phaseTwo.parse();
if (c.checkCommand(BY)) {
return Commands.of(mainCmd, task, c);
} else {
// Wrong format
throw new NullPointerException();
}
} catch (DukeUnknownCommandException | NullPointerException e) {
// Wrong format
throw new DukeNullPointerException("The format for the command is: "
+ "deadline task /by date&time");
}
case EVENT:
try {
String task = this.commandPhaseParse();
String secondaryCommand = this.phaseTwo();
String tertiaryCommand = this.phaseThree();
if (task == null) {
throw new DukeException("Please add the task name");
}
Parser phaseTwo = new Parser(secondaryCommand);
Commands secCmd = phaseTwo.parse();
Parser phaseThree = new Parser(tertiaryCommand);
Commands terCmd = phaseThree.parse();
if (!secCmd.compareTime(terCmd)) {
throw new DukeFromEarlierThanToException("From must be earlier than To");
}
if (secCmd.checkCommand(FROM) && terCmd.checkCommand(TO)) {
return Commands.of(mainCmd, task, secCmd, terCmd);
} else {
// Wrong format
throw new NullPointerException();
}
} catch (DukeUnknownCommandException | NullPointerException e) {
// Wrong format
throw new DukeNullPointerException("The format for the command is: "
+ "event task /from startDayDateTime /to endDayDateTime");
}
default:
// Default would be unknown command, but that is an exception, thus it will be thrown.
}
throw new DukeUnknownCommandException("Unknown command");
}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/parser/Parser.java lines 29-33:
/**
* Construct the Parser object.
*
* @param command The string that needs to be parsed.
*/Example from src/main/java/task/ListOfTask.java lines 21-25:
/**
* The size of the task list.
*
* @return Returns the size of the task list.
*/Example from src/main/java/task/ListOfTask.java lines 41-47:
/**
* Undo the previous delete command.
*
* @param index The index of the task that is to be added back.
* @return Returns a string showing that the task has been added back.
* @throws DukeSaveException If Duke is unable to save the task list.
*/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 2dffb9d:
Refactoring and Code Quality
- Not in imperative mood (?)
possible problems in commit e51fc49:
Fixed javaFx and made the GUI
- Not in imperative mood (?)
Suggestion: Follow the given conventions for Git commit messages for future commits (no need to modify past commit messages).
Aspect: Binary files in repo
No easy-to-detect 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.