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

[soc-se-bot]

@cashewnade 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/aeromon/CommandManager.java lines 19-86:

    public static Command read(String fullCommand) throws AeromonException {
        assert fullCommand != null : "Command is null";

        String[] parameters = fullCommand.split(" ", 2);
        String command = parameters[0];

        switch (command) {
            case "bye":
                return new ByeCommand();

            case "list":
                return new ListCommand();

            case "mark": {
                int taskNum = getTaskNum(parameters[1]);
                return new EditCommand(EditCommand.EditType.MARK, taskNum);
            }

            case "unmark": {
                int taskNum = getTaskNum(parameters[1]);
                return new EditCommand(EditCommand.EditType.UNMARK, taskNum);
            }

            case "delete": {
                int taskNum = getTaskNum(parameters[1]);
                return new EditCommand(EditCommand.EditType.DELETE, taskNum);
            }

            case "todo": {
                return new AddCommand(AddCommand.TaskType.TODO, new String[]{checkDescription("Todo", parameters[1])});
            }

            case "deadline": {
                String deadlineDescription = checkDescription("Deadline", parameters[1]);
                String[] deadlineTokens = deadlineDescription.split(" /by ");
                return new AddCommand(AddCommand.TaskType.DEADLINE, deadlineTokens);
            }

            case "event": {
                String eventDescription = checkDescription("Event", parameters[1]);
                String[] eventTokens = eventDescription.split(" /at ");
                return new AddCommand(AddCommand.TaskType.EVENT, eventTokens);
            }

            case "ftodo": {
                return new ForceAddCommand(AddCommand.TaskType.TODO, new String[]{checkDescription("Todo", parameters[1])});
            }

            case "fdeadline": {
                String deadlineDescription = checkDescription("Deadline", parameters[1]);
                String[] deadlineTokens = deadlineDescription.split(" /by ");
                return new ForceAddCommand(AddCommand.TaskType.DEADLINE, deadlineTokens);
            }

            case "fevent": {
                String eventDescription = checkDescription("Event", parameters[1]);
                String[] eventTokens = eventDescription.split(" /at ");
                return new ForceAddCommand(AddCommand.TaskType.EVENT, eventTokens);
            }

            case "find": {
                return new FindCommand(parameters[1]);
            }

            default:
                throw new AeromonException("Nani? Me no understand what you say .-.");
        }
    }

Example from src/main/java/aeromon/Storage.java lines 38-97:

    public ArrayList<Task> getFile() throws AeromonException {

        try {
            if (localTasks.createNewFile()) {
                return new ArrayList<>();
            } else {
                ArrayList<Task> tasks = new ArrayList<>();
                Scanner fileScanner = new Scanner(localTasks);
                DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofPattern("MMM dd yyyy");

                while (fileScanner.hasNextLine()) {
                    String content = fileScanner.nextLine();
                    String[] tokens = content.split(" / ");

                    switch (tokens[0]) { //tokens[0] indicates the task type
                    case "T":
                        ToDo toDo;

                        if (tokens[1].equals("1")) {
                            toDo = new ToDo(tokens[2], true);
                        } else {
                            toDo = new ToDo(tokens[2], false);
                        }

                        tasks.add(toDo);
                        break;

                    case "D":
                        Deadline deadline;

                        if (tokens[1].equals("1")) {
                            deadline = new Deadline(tokens[2], true, LocalDate.parse(tokens[3], dateTimeFormatter));
                        } else {
                            deadline = new Deadline(tokens[2], false, LocalDate.parse(tokens[3], dateTimeFormatter));
                        }

                        tasks.add(deadline);
                        break;

                    case "E":
                        Event event;

                        if (tokens[1].equals("1")) {
                            event = new Event(tokens[2], true, LocalDate.parse(tokens[3], dateTimeFormatter));
                        } else {
                            event = new Event(tokens[2], false, LocalDate.parse(tokens[3], dateTimeFormatter));
                        }

                        tasks.add(event);
                        break;

                    }
                }
                fileScanner.close();
                return tasks;
            }
        } catch (IOException e) {
            throw new AeromonException("The file had some error and I wasn't able to read it properly >.<");
        }
    }

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

No easy-to-detect issues 👍

Aspect: Recent Git Commit Message (Subject Only)

possible problems in commit c8665c2:

Updated User Guide

• Not in imperative mood (?)

possible problems in commit b65b612:

Updated code based on Code Quality Feedback

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