Sharing iP code quality feedback [for @Linxcathyyy] - Round 2 #4
Description
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/meow/TaskList.java lines 140-140:
boolean resultFound = false;Suggestion: Follow the given naming convention for boolean variables/methods (e.g., use a boolean-soundingprefix).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/meow/Meow.java lines 43-100:
private String processCommand(String input) throws MeowException {
try {
Command userCommand = this.parser.checkCommand(input);
switch (userCommand) {
case BYE:
System.exit(0);
return this.ui.exit();
case LIST:
return this.ui.displayList(this.tasks.getTasksList());
case DONE:
String output = this.tasks.completeTask(this.parser.getTaskNumber(input));
this.storage.addArrayTaskToFile(this.tasks.getTasksList());
return output;
case FIND:
List<Task> filteredTasks = this.tasks.searchTask(this.parser.getTaskNumber(input));
return this.ui.displaySearchList(filteredTasks);
case TODO:
String todoTask = this.parser.getTask(input, userCommand);
Todo newTodo = this.tasks.addTodo(todoTask);
this.storage.addArrayTaskToFile(this.tasks.getTasksList());
return this.ui.printTaskList(this.tasks.getTasksList(), newTodo);
case EVENT:
String eventTask = this.parser.getTask(input, userCommand);
String[] eventTaskAndDate = this.parser.getTaskAndDate(eventTask, userCommand);
try {
Event newEvent = this.tasks.addEvent(eventTaskAndDate[0], eventTaskAndDate[1]);
this.storage.addArrayTaskToFile(this.tasks.getTasksList());
return this.ui.printTaskList(this.tasks.getTasksList(), newEvent);
} catch (ArrayIndexOutOfBoundsException exception) {
throw new EmptyEventTimeException();
}
case DEADLINE:
String deadlineTask = this.parser.getTask(input, userCommand);
String[] deadlineTaskAndDate = this.parser.getTaskAndDate(deadlineTask, userCommand);
try {
String[] dateAndTime = deadlineTaskAndDate[1].split(" ");
if (this.parser.isLocalDateTime(dateAndTime[0], dateAndTime[1])) {
LocalDate date = this.parser.convertToLocalDate(dateAndTime[0]);
Deadline newDeadline = this.tasks.addDeadline(deadlineTaskAndDate[0], date, dateAndTime[1]);
this.storage.addArrayTaskToFile(this.tasks.getTasksList());
return this.ui.printTaskList(this.tasks.getTasksList(), newDeadline);
} else {
throw new InvalidDateTimeException();
}
} catch (ArrayIndexOutOfBoundsException exception) {
throw new EmptyDeadlineTimeException();
}
case DELETE:
String deleteOutput = this.tasks.deleteTask(this.parser.getTaskNumber(input));
this.storage.addArrayTaskToFile(this.tasks.getTasksList());
return deleteOutput;
default:
throw new InvalidInputException();
}
} catch (IllegalArgumentException e) {
throw new InvalidInputException();
}
}Example from src/main/java/meow/TaskList.java lines 139-184:
public List<Task> searchTask(String ... keywords) {
boolean resultFound = false;
List<Task> filteredTasks = new ArrayList<>();
final String TODO = "todo";
final String EVENT = "event";
final String DEADLINE = "deadline";
for (int i = 0; i < this.tasksList.size(); i++) {
Task task = this.tasksList.get(i);
String description = task.getDescription();
String status = task.getStatusIcon();
String typeOfTask = "";
String time = "";
String date = "";
if (task instanceof Event) {
time = ((Event) task).getEventTime();
typeOfTask = EVENT;
} else if (task instanceof Deadline) {
time = ((Deadline) task).getDeadlineTime();
typeOfTask = DEADLINE;
date = ((Deadline) task).getDeadlineDate();
System.out.println(date);
} else {
typeOfTask = TODO;
}
for (int j = 0; j < keywords.length; j++) {
resultFound = resultFound || description.contains(keywords[j].toLowerCase(Locale.ROOT))
||
status.contains(keywords[j])
||
time.contains(keywords[j].toLowerCase(Locale.ROOT))
||
typeOfTask.contains(keywords[j].toLowerCase(Locale.ROOT))
||
date.contains(keywords[j].toLowerCase(Locale.ROOT));
}
if (resultFound) {
filteredTasks.add(this.tasksList.get(i));
}
resultFound = false;
}
return filteredTasks;
}Suggestion: Consider applying SLAP (and other abstraction mechanisms) to shorten 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/meow/Launcher.java lines 10-14:
/**
* Start of the application.
*
* @param args The given args.
*/Example from src/main/java/meow/TaskList.java lines 132-138:
/**
* Filter the list of meow.Task objects based on the keyword provided by the user,
* and returns a list of meow.Task objects.
*
* @param keywords The search keywords input by the user.
* @return A list of filtered meow.Task objects.
*/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 (Subject Only)
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 @nus-se-bot used to post this issue is un-manned. Do not reply to this post (as those replies will not be read). Instead, contact cs2103@comp.nus.edu.sg if you want to follow up on this post.