Sharing iP code quality feedback [for @chiangqinkang] - Round 2

[soc-se-bot-blue]

@chiangqinkang 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

No easy-to-detect issues 👍

Aspect: Brace Style

No easy-to-detect issues 👍

Aspect: Package Name Style

Example from src/main/java/CommandLine/Line.java lines 1-1:

package CommandLine;

Example from src/main/java/CommandLine/NotAGPT.java lines 1-1:

package CommandLine;

Example from src/main/java/CommandLine/Parser.java lines 1-1:

package CommandLine;

Suggestion: Follow the package naming convention specified by the coding standard.

Aspect: Class Name Style

No easy-to-detect issues 👍

Aspect: Dead Code

Example from src/test/java/test.java lines 1-1:

//import java.time.LocalDateTime;

Example from src/test/java/test.java lines 2-2:

//import java.time.format.DateTimeFormatter;

Example from src/test/java/test.java lines 3-3:

//import java.time.format.DateTimeParseException;

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/CommandLine/Parser.java lines 20-84:

    public static String parse(TaskList taskList, String inputTokens) {
        Line line = new Line();
        String[] parts = inputTokens.split(" ", 2);
        String command = parts[0].toLowerCase();
        switch(command) {
        case "bye":
            NotAGPTExit();
            return "Goodbye";
        case "list":
            return taskList.list();
        case "mark":
            if (parts.length > 1) {
                String idx = parts[1];
                return taskList.markAsDone(idx);
            } else {
                return "Enter a task number";
            }
        case "unmark":
            if (parts.length > 1) {
                String idx = parts[1];
                return taskList.markAsUndone(idx);
            } else {
                return "Enter a task number";
            }
        case "todo":
            if (parts.length > 1) {
                return taskList.add(parts[1], Task.TaskType.T);
            } else {
                return "Enter a name for the To Do Task";
            }
        case "deadline":
            if (parts.length > 1) {
                return taskList.add(parts[1], Task.TaskType.D);
            } else {
                return "Incomplete command. Enter a deadline";
            }
        case "event":
            if (parts.length > 1) {
                return taskList.add(parts[1], Task.TaskType.E);
            } else {
                return "Incomplete command. Enter a start and end time";
            }
        case "delete":
            if (parts.length == 2) {
                try {
                    int idx = parseInt(parts[1]);
                    return taskList.delete(idx);
                } catch (NumberFormatException e) {
                    return "Enter a valid index to delete";
                }
            } else {
                return "Enter a valid argument";
            }
        case "find":
            if (parts.length == 2) {
                String word = parts[1];
                return taskList.find(word);
            } else {
                return "Enter a word";
            }
        default:
            return "Unknown command, refer to the user guide for list of commands";

        }
    }

Example from src/main/java/Task/TaskList.java lines 121-166:

    public String add(String s, Task.TaskType taskType) {
        StringBuilder response = new StringBuilder();
        try {
            Task newTask;
            switch (taskType) {
                case T:
                    newTask = ToDo.of(s, taskType);
                    break;
                case D:
                    newTask = Deadline.of(s, taskType);
                    break;
                case E:
                    newTask = Event.of(s, taskType);
                    break;
                default:
                    throw new IllegalArgumentException("Invalid task type");
            }
            if (isDuplicate(newTask)) {
                return "That task is a duplicate, unable to add task";
            }
            this.taskList.add(newTask);
            saveTasks();
            this.taskListLength += 1;

            response.append("Got it. I've added this task:\n")
                    .append("[")
                    .append(newTask.getTaskTypeAsString())
                    .append("][ ] ")
                    .append(newTask.getTaskDescription())
                    .append("\n");

            if (this.taskListLength == 1) {
                response.append("Now you have 1 task in the list.\n");
            } else {
                response.append("Now you have ")
                        .append(this.taskListLength)
                        .append(" tasks in the list.\n");
            }

        } catch (TaskCreationException e) {
            response.append(e.getMessage())
                    .append("\n");
        }

        return response.toString();
    }

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

No easy-to-detect issues 👍

Aspect: Recent Git Commit Message

possible problems in commit a8d28fd:

Fix issues with saving

This fixed an issue with saving tasks. The issue with loading tasks from the save file was also fixed.

• body not wrapped at 72 characters: e.g., This fixed an issue with saving tasks. The issue with loading tasks from the save file was also fixed.

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 cs2103@comp.nus.edu.sg if you want to follow up on this post.