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

[nus-se-bot]

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

Example from src/main/java/lele/task/TaskList.java lines 177-177:

        // assert this.listSize() >= inputIndex : "Index parameter queried should be within the task size";

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/lele/parser/Parser.java lines 54-148:

    public static Command parse(String fullCommand) throws UnknownCommandException, EmptyDescException {
        try {
            String[] inputArr = fullCommand.split(" ");
            Commands command = Commands.valueOf(inputArr[0].toUpperCase());
            String desc = "variable not initialised";
            StringBuilder sb = new StringBuilder();
            switch (command) {
            case TODO:
                if (inputArr.length == 1) {
                    throw new EmptyDescException("OOPS!!! The description of a todo cannot be empty.");
                }
                for (int i = 1; i < inputArr.length; i++) {
                    sb.append(inputArr[i]);
                    if (i != inputArr.length - 1) {
                        sb.append(" ");
                    }
                }
                Todo todo = new Todo(sb.toString().trim());
                return new TodoCommand(todo);
            case DEADLINE:
                if (inputArr.length == 1) {
                    throw new EmptyDescException("OOPS!!! The description of a deadline cannot be empty.");
                }
                for (int i = 1; i < inputArr.length; i++) {
                    if (inputArr[i].equals("/by")) {
                        desc = sb.toString();
                        sb.setLength(0);
                        continue;
                    }
                    sb.append(inputArr[i]);
                    if (i != inputArr.length - 1) {
                        sb.append(" ");
                    }
                }
                String[] dateTime = sb.toString().split(" ");
                assert !desc.isEmpty() : "desc (deadline description) should be filled, otherwise caught by exception";
                Deadline deadline = new Deadline(desc, dateTime[0], dateTime[1]);
                return new DeadlineCommand(deadline);
            case EVENT:
                if (inputArr.length == 1) {
                    throw new EmptyDescException("OOPS!!! The description of an event cannot be empty.");
                }
                String from = "local variable not initialised";
                for (int i = 1; i < inputArr.length; i++) {
                    if (inputArr[i].equals("/from")) {
                        desc = sb.toString();
                        sb.setLength(0);
                        continue;
                    } else if (inputArr[i].equals("/to")) {
                        from = sb.toString();
                        sb.setLength(0);
                        continue;
                    }
                    sb.append(inputArr[i]);
                    if (i != inputArr.length - 1) {
                        sb.append(" ");
                    }
                }
                assert !desc.isEmpty() : "desc (event description) should be filled, otherwise caught by exception";
                assert !from.isEmpty() : "from (event starting date) should be filled";
                Event event = new Event(desc, from, sb.toString().trim());
                return new EventCommand(event);
            case MARK:
                index = Integer.parseInt(inputArr[1]);
                return new MarkCommand(index);
            case UNMARK:
                index = Integer.parseInt(inputArr[1]);
                return new UnmarkCommand(index);
            case DELETE:
                index = Integer.parseInt(inputArr[1]);
                return new DeleteCommand(index);
            case LIST:
                return new ListCommand();
            case BYE:
                return new ByeCommand();
            case FIND:
                if (inputArr.length == 1) {
                    throw new EmptyDescException("OOPS!!! You need to specify what you want to find!");
                }
                return new FindCommand(inputArr[1]);
            case UNDO:
                if (inputArr.length == 1) {
                    throw new EmptyDescException("OOPS!!! Please specify how many CRUD versions you want to undo!");
                }
                int index = Integer.parseInt(inputArr[1]);
                return new UndoCommand(index);
            default:
                assert false; // Execution should never reach this point!
                throw new UnknownCommandException("OOPS!!! You need to give me an input :-(");
            }
        } catch (IllegalArgumentException e) {
            throw new UnknownCommandException("OOPS!!! I'm sorry, but I don't know what that means :-(");
        }

    }

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/lele/Lele.java lines 65-69:

    /**
     * The main method, initialises the program instance.
     *
     * @param args Takes in the command line argument.
     */

Example from src/main/java/lele/task/Deadline.java lines 43-47:

    /**
     * The date and time of the deadline, to be stored in the user's data.
     *
     * @return A string representation of the date and time of the deadline.
     */

Example from src/main/java/lele/task/Event.java lines 34-38:

    /**
     * The starting date and time of the event.
     *
     * @return String representation of the starting date and time set.
     */

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)

possible problems in commit bdf730f:

Changed UG images folder name

• 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 👍

❗ 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.