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

[nus-se-bot]

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

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/jeff/parser/Parser.java lines 32-99:

    public static Command parse(String fullCommand) throws JeffException {
        String[] splitCommand = fullCommand.split(" ", 2);
        String keyword = splitCommand[0];
        String body = checkValidInfo(splitCommand);
        int len = splitCommand.length;

        // Perform the correct instructions according to the keyword.
        switch(keyword) {
        case ("bye"):
            isExit = true;
            return new ByeCommand();
        case ("list") :
            return new ListCommand();
        case ("mark"):
            checkValidFirstEntry(len, " ☹ OOPS!!! Please tell me the task's"
                    + " index number so that I can mark it as done.");
            return new MarkCommand(body);
        case ("unmark"):
            checkValidFirstEntry(len, " ☹ OOPS!!! Please tell me the task's"
                    + " index number so that I can mark it as not done.");
            return new UnmarkCommand(body);
        case ("todo"):
            checkValidFirstEntry(len, " ☹ OOPS!!! The description of a todo cannot be empty.");
            return new TodoCommand(body);
        case ("deadline"):
            checkValidFirstEntry(len, " ☹ OOPS!!! The description of a deadline cannot be empty.");

            String[] splitBody = body.split(" /by ", 2);
            String description = splitBody[0];
            String dateInfo = checkValidInfo(splitBody);
            int bodyLength = splitBody.length;

            checkValidDateTime(bodyLength, dateInfo, " ☹ OOPS!!! Please input a due date for this task");
            return new DeadlineCommand(description, dateInfo);
        case ("event"):
            checkValidFirstEntry(len, " ☹ OOPS!!! The description of a event cannot be empty.");

            splitBody = body.split(" /at ", 2);
            description = splitBody[0];
            dateInfo = checkValidInfo(splitBody);
            bodyLength = splitBody.length;

            checkValidDateTime(bodyLength, dateInfo, " ☹ OOPS!!! Please input a event date");
            return new EventCommand(description, dateInfo);
        case ("delete"):
            checkValidFirstEntry(len, " ☹ OOPS!!! Please tell me the task's"
                    + " index number so that I can delete it from the list.");
            return new DeleteCommand(body);
        case ("find"):
            checkValidFirstEntry(len, " ☹ OOPS!!! Please tell me the keyword"
                    + " so that I know what you are looking for.");
            return new FindCommand(body);
        case ("note"):
            checkValidFirstEntry(len, " ☹ OOPS!!! Please tell me what you want"
                    + " to be added into the notes.");
            switch(body) {
            case ("clear"):
                return new ClearNoteCommand();
            case ("list"):
                return new ListNoteCommand();
            default:
                break;
            }
            return new AddNoteCommand(body);
        default:
            return new HelpCommand();
        }
    }

Example from src/main/java/jeff/storage/Storage.java lines 39-103:

    public TaskList load() throws JeffException {

        File f = new File(filePath);
        TaskList tasks = new TaskList();

        // When the file or directory does not exist, create it.
        Scanner sc;
        try {
            sc = new Scanner(f);
        } catch (FileNotFoundException e) {
            File parent = f.getParentFile();
            if (!parent.exists()) {
                parent.mkdir();
            }
            return tasks;
        }

        // Look through every line on the external file and add to TaskList.
        while (sc.hasNext()) {
            String input = sc.nextLine();
            try {
                String[] inputLine = input.split("\\| ", 4);
                String type = inputLine[0];
                String done = inputLine[1];
                String name = inputLine[2];
                String time;

                // New task from the current line
                Task curr;
                // Check what type of task is this.
                switch (type) {
                case "T ":
                    curr = new Todo(name);
                    break;
                case "D ":
                    time = inputLine[3];
                    curr = new Deadline(name.substring(0, name.length() - 1), time);
                    break;
                case "E ":
                    time = inputLine[3];
                    curr = new Event(name.substring(0, name.length() - 1), time);
                    break;
                default:
                    curr = null;
                }
                // Check if the task is marked as done.
                switch (done) {
                case "1 ":
                    curr.setMark();
                    break;
                case "0 ":
                    break;
                default:
                    break;
                }
                // After all the necessary information, add it into the TaskList.
                tasks.add(curr);
            } catch (DateTimeParseException e) {
                throw new JeffException("Invalid date time format at: " + input
                        + "Please refer to readme.txt for the available formats");
            }
        }
        sc.close();
        return tasks;
    }

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/jeff/main/Jeff.java lines 38-45:

    /**
     * Get the response for Jeff to display in the GUI, response can be
     * either a confirmation that the intended task is done, or in the case an exception
     * was thrown, display what went wrong.
     *
     * @param input user input.
     * @return Jeff's response.
     */

Example from src/main/java/jeff/note/Note.java lines 28-30:

    /**
     * Delete all entries in the note.
     */

Example from src/main/java/jeff/parser/Parser.java lines 114-119:

    /**
     * Prevent NullPointerException when declaring a variable that doesn't exist.
     *
     * @param arr Contains information of the desired return value, if it exist.
     * @return Desired output if it exists.
     */

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 👍

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.