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

[nus-se-bot]

@rahula1008 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/sirpotato/SortCommand.java lines 14-14:

        //System.out.println(SORTED_LIST_MSG + ui.listTasks(tasks.sortBy(categoryToSortBy)));

Example from src/main/java/sirpotato/TaskList.java lines 78-78:

        // ui.displayDeletionMessage(toDoList.get(taskNumber), toDoList);

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/sirpotato/Parser.java lines 32-126:

    public static void checkForErrors(String userInput) throws DukeException {
        if (userInput.startsWith("todo")) {
            if (!userInput.startsWith("todo ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 5 || userInput.substring(5).isEmpty()) {
                throw new DukeException("Mate, you have got to give us a description of the task");
            }
        } else if (userInput.startsWith("deadline")) {
            if (!userInput.startsWith("deadline ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 9) {
                throw new DukeException("Mate, you need to give me a task description and deadline date");
            }
            String[] sectionedString = userInput.split("/by ");
            if (sectionedString.length < 2 || 
                sectionedString[0].substring(9).isEmpty() || 
                sectionedString[1].isEmpty()) {
                throw new DukeException("Mate, you need to give me a task description and deadline date");
            }
            validateDateFormat(sectionedString[1].trim());
        } else if (userInput.startsWith("event")) {
            if (!userInput.startsWith("event ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 6) {
                throw new DukeException("Mate, an event should have the description, the start, and the end.");
            }
            String[] sectionedString = userInput.split(" /from | /to ");
            if (sectionedString.length < 3 || sectionedString[0].substring(6).isEmpty() ||
                sectionedString[1].isEmpty() || sectionedString[2].isEmpty()) {
                throw new DukeException("Mate, an event should have the description, the start, and the end.");
            }
            validateDateFormat(sectionedString[1].trim());
            validateDateFormat(sectionedString[2].trim());
        } else if (userInput.startsWith("delete")) {
            if (!userInput.startsWith("delete ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 7) {
                throw new DukeException("You need to say which item to delete");
            } 
            String[] sectionedString = userInput.trim().split("\\s+");
            String taskToDeleteString = sectionedString[1].trim();
            if (!taskToDeleteString.matches("\\d+")) {
                throw new DukeException("Mate, please give a task number(not text) to delete");
            }
        } else if (userInput.startsWith("sort")) {
            if (!userInput.startsWith("sort ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 5) {
                throw new DukeException("Mate, please specify your category: either description or deadline");
            }
            String[] sectionedString = userInput.split(" ");
            String categoryToSortBy = sectionedString[1];
            if (!(categoryToSortBy.equals("description") || categoryToSortBy.equals("deadline"))) {
                throw new DukeException("You have to sort by description or deadline");
            }
        } else if (userInput.trim().startsWith("mark")) {
            if (!userInput.startsWith("mark ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 5) {
                throw new DukeException("Mate, please specify your task to mark");
            }
            String[] sectionedString = userInput.trim().split("\\s+");
            String taskToMarkString = sectionedString[1].trim();
            if (!taskToMarkString.matches("\\d+")) {
                throw new DukeException("Mate, please give a task number(not text) to mark");
            }
        } else if (userInput.trim().startsWith("unmark")) {
            if (!userInput.startsWith("unmark ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 7) {
                throw new DukeException("Mate, please specify your task to mark");
            }
            String[] sectionedString = userInput.trim().split("\\s+");
            String taskToUnmarkString = sectionedString[1].trim();
            if (!taskToUnmarkString.matches("\\d+")) {
                throw new DukeException("Mate, please give a task number(not text) to unmark");
            }
        } else if (userInput.trim().startsWith("find")) {
            if (!userInput.startsWith("find ")) {
                throw new DukeException("Mate, you may have misspelt or forgotten to type the rest of your command");
            }
            if (userInput.trim().length() <= 5) {
                throw new DukeException("Mate, please specify a keyword/string to search for");
            }
        }else if (!userInput.trim().equals("bye") && !userInput.trim().equals("list")) {
            throw new DukeException("I'm sorry, that is not a valid input");
        }
    }

Example from src/main/java/sirpotato/Parser.java lines 146-187:

    public static Command parseCommand(String userInput) throws DukeException {
        checkForErrors(userInput);

        if (userInput.trim().equals("bye")) {
            return new ExitCommand();
        } else if (userInput.trim().equals("list")) {
            return new ListCommand();
        } else if (userInput.startsWith("mark")) {
            int itemNumber = Integer.parseInt(userInput.split(" ")[1]) - 1;
            return new MarkCommand(itemNumber);
        } else if (userInput.startsWith("unmark")) {
            int itemNumber = Integer.parseInt(userInput.split(" ")[1]) - 1;
            return new UnmarkCommand(itemNumber);
        } else if (userInput.startsWith("todo")) {
            String description = userInput.substring(5);
            return new AddCommand(new Todo(description));
        } else if (userInput.startsWith("find")) {
            String searchString = userInput.substring(5);
            return new FindCommand(searchString);
        }else if (userInput.startsWith("deadline")) {
            String[] sectionedString = userInput.split("/by ");
            String description = sectionedString[0].substring(9);
            LocalDate by = parseDate(sectionedString[1].trim());
            return new AddCommand(new Deadline(description, by));
        } else if (userInput.startsWith("event")) {
            String[] sectionedString = userInput.split(" /from | /to ");
            String description = sectionedString[0].substring(6);
            LocalDate from = parseDate(sectionedString[1]);
            LocalDate to = parseDate(sectionedString[2]);
            return new AddCommand(new Event(description, from, to));
        } else if (userInput.startsWith("delete")) {
            String[] sectionedString = userInput.split(" ");
            int itemToDelete = Integer.parseInt(sectionedString[1]) - 1;
            return new DeleteCommand(itemToDelete);
        } else if (userInput.startsWith("sort")) {
            String[] sectionedString = userInput.split(" ");
            String categoryToSortBy = sectionedString[1];
            return new SortCommand(categoryToSortBy);
        } else {
            throw new DukeException("That is not valid input mate. Please have another go.");
        }
    }

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/sirpotato/Ui.java lines 119-126:

    /**
     * When a user marks an item complete, this is the message that will be displayed
     * Prints and returns the marked item
     * 
     * @param itemNumber the index number(starts at 1) of the item to be deleted
     * @param toDoList the TaskList's arrayList containing the current to-do list.
     * @return the marked item message
     */

Example from src/main/java/sirpotato/Ui.java lines 134-141:

    /**
     * When a user unmarks an item to incomplete, this is the message that will be displayed
     * Prints and returns unmarked item  
     * 
     * @param itemNumber the index number(starts at 1) of the item to be deleted
     * @param toDoList the TaskList's arrayList containing the current to-do list.
     * @return the unmarked item message
     */

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

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.