Sharing iP code quality feedback [for @rahula1008]

[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, 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

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/Storage.java lines 33-33:

            //File f = new File("../../../data/list.txt"); 

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-71:

    public static void checkForErrors(String userInput) throws DukeException {
        if (userInput.startsWith("todo")) {
            if (userInput.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")) {
            String[] sectionedString = userInput.split("/by ");
            if (sectionedString.length < 2 || 
                sectionedString[0].substring(9).isEmpty() || 
                sectionedString[1].isEmpty()) {
                throw new DukeException("Mate, a deadline must include a task, and the deadline");
            }
            validateDateFormat(sectionedString[1].trim());
        } else if (userInput.startsWith("event")) {
            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.length() <= 7) {
                throw new DukeException("You need to say which item to delete");
            } 
        } else if (userInput.startsWith("sort")) {
            if (userInput.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.equals("bye") && !userInput.equals("list") && 
                   !userInput.startsWith("mark") && !userInput.startsWith("unmark")
                   && !userInput.startsWith("find")) {
            throw new DukeException("I'm sorry, that is not a valid input");
        }
    }

Example from src/main/java/sirpotato/Parser.java lines 91-132:

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

        if (userInput.equals("bye")) {
            return new ExitCommand();
        } else if (userInput.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

possible problems in commit 843ed03:

Remove horizontal line and indents from UI

The UI class displays horizontal lines and indents from when it was command-line based

As a step towards updating the UI to fit the new GUI, let's change the display methods to just display the necesary text instead of the horizontal lines and indents.

• body not wrapped at 72 characters: e.g., The UI class displays horizontal lines and indents from when it was command-line based

Suggestion: Follow the given conventions for Git commit messages for future commits (do not modify past commit messages as doing so will change the commit timestamp that we used to detect your commit timings).

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