Sharing iP code quality feedback [for @kimsianglim]

[nus-se-bot]

@kimsianglim We did an automated analysis of your code to detect potential areas to improve the code quality. We are sharing the results below, to help you improve the iP code further.

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/nova/Nova.java lines 82-145:

    private boolean execute(Command cmd) throws NovaException, IOException {
        switch (cmd.getType()) {
        case EXIT:
            ui.showBye();
            return true;

        case LIST:
            ui.showListHeader();
            for (int i = 0; i < tasks.size(); i++) {
                ui.showListItem(i + 1, tasks.get(i).toString());
            }
            ui.showListFooter();
            storage.saveTasks(tasks.getTasks());
            return false;

        case MARK:
            tasks.mark(cmd.getIndex());
            storage.saveTasks(tasks.getTasks());
            ui.showTaskMarked(tasks.get(cmd.getIndex()).toString());
            return false;

        case UNMARK:
            tasks.unmark(cmd.getIndex());
            storage.saveTasks(tasks.getTasks());
            ui.showTaskUnmarked(tasks.get(cmd.getIndex()).toString());
            return false;

        case DELETE:
            Task removed = tasks.remove(cmd.getIndex());
            storage.saveTasks(tasks.getTasks());
            ui.showTaskDeleted(removed.toString(), tasks.size());
            return false;

        case TODO:
            tasks.add(new ToDo(cmd.getDescription()));
            storage.saveTasks(tasks.getTasks());
            ui.showTaskAdded(tasks.get(tasks.size() - 1).toString(), tasks.size());
            return false;

        case DEADLINE:
            tasks.add(new Deadline(cmd.getDescription(), cmd.getBy()));
            storage.saveTasks(tasks.getTasks());
            ui.showTaskAdded(tasks.get(tasks.size() - 1).toString(), tasks.size());
            return false;

        case EVENT:
            tasks.add(new Event(cmd.getDescription(), cmd.getFrom(), cmd.getTo()));
            storage.saveTasks(tasks.getTasks());
            ui.showTaskAdded(tasks.get(tasks.size() - 1).toString(), tasks.size());
            return false;

        case FIND:
            ArrayList<Integer> matches = tasks.findIndexes(cmd.getDescription());
            ui.showFindHeader();
            for (int idx : matches) {
                ui.showListItem(idx + 1, tasks.get(idx).toString());
            }
            ui.showListFooter();
            return false;

        default:
            throw new NovaException("So sorry, I don't understand what that means.");
        }
    }

Example from src/main/java/nova/Parser.java lines 32-105:

    public static Command parse(String input) throws NovaException {
        if (input.equals("bye")) {
            return Command.exit();
        }

        if (input.equals("list")) {
            return Command.list();
        }

        if (input.startsWith("mark ")) {
            int idx = parseIndex(input.substring(5), "mark needs a task number, e.g. mark 1");
            return Command.mark(CommandType.MARK, idx);
        }

        if (input.startsWith("unmark ")) {
            int idx = parseIndex(input.substring(7), "unmark needs a task number, e.g. unmark 1");
            return Command.mark(CommandType.UNMARK, idx);
        }

        if (input.startsWith("delete ")) {
            int idx = parseIndex(input.substring(7), "delete needs a task number, e.g. delete 1");
            return Command.delete(idx);
        }

        if (input.equals("todo") || input.startsWith("todo ")) {
            String desc = input.length() > 4 ? input.substring(4).trim() : "";
            if (desc.isEmpty()) {
                throw new NovaException("The description of a todo cannot be empty.");
            }
            return Command.todo(desc);
        }

        if (input.startsWith("deadline ")) {
            int byIndex = input.indexOf(" /by ");
            if (byIndex == -1) {
                throw new NovaException("follow this format: deadline <description> /by <time>");
            }

            String desc = input.substring(9, byIndex).trim();
            String by = input.substring(byIndex + 5).trim();
            if (desc.isEmpty() || by.isEmpty()) {
                throw new NovaException("description and by time cannot be empty.");
            }

            return Command.deadline(desc, by);
        }

        if (input.startsWith("event ")) {
            int fromIndex = input.indexOf(" /from ");
            int toIndex = input.indexOf(" /to ");
            if (fromIndex == -1 || toIndex == -1 || toIndex < fromIndex) {
                throw new NovaException("follow this format: event <description> /from <start> /to <end>");
            }

            String desc = input.substring(6, fromIndex).trim();
            String from = input.substring(fromIndex + 7, toIndex).trim();
            String to = input.substring(toIndex + 5).trim();
            if (desc.isEmpty() || from.isEmpty() || to.isEmpty()) {
                throw new NovaException("description and time cannot be empty.");
            }

            return Command.event(desc, from, to);
        }

        if (input.equals("find") || input.startsWith("find ")) {
            String keyword = input.length() > 4 ? input.substring(4).trim() : "";
            if (keyword.isEmpty()) {
                throw new NovaException("The keyword for find cannot be empty.");
            }
            return Command.find(keyword);
        }

        throw new NovaException("So sorry, I don't understand 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

No easy-to-detect issues 👍

Aspect: Recent Git Commit Messages

No easy-to-detect issues 👍

Aspect: Binary files in repo

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