Sharing iP code quality feedback [for @kahleongq]

[nus-se-bot]

@kahleongq 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/duke/Parser.java lines 16-134:

    public static Command parse(String command, Ui ui) {
        command = command.strip().replaceAll(" +", " ");
        String firstWord = command.split(" ")[0];
        String description;
        switch (firstWord) {
        case "bye":
            try {
                if (!command.equals("bye")) {
                    throw new DukeException("No words should be after 'bye'.");
                }
                return new ExitCommand(firstWord);
            } catch (DukeException e) {
                ui.showMessage(e.getMessage());
            }
            break;
        case "list":
            try {
                if (!command.equals("list")) {
                    throw new DukeException("No words should be after 'list'.");
                }
                return new ListCommand(firstWord);
            } catch (DukeException e) {
                ui.showMessage(e.getMessage());
            }
            break;
        case "delete":
            try {
                if (command.equals("delete")) {
                    throw new DukeException("Please type what you want to delete!");
                }
                description = command.substring(firstWord.length() + 1);
                int taskNum = Integer.parseInt(description);
                return new DeleteCommand(firstWord, taskNum);
            } catch (NumberFormatException e) {
                ui.showMessage("Please input an integer!");
            } catch (DukeException e) {
                ui.showMessage(e.getMessage());
            }
            break;
        case "mark":
            try {
                if (command.equals("mark")) {
                    throw new DukeException("Please type what you want to mark!");
                }
                description = command.substring(firstWord.length() + 1);
                int taskNum = Integer.parseInt(description);
                return new MarkCommand(firstWord, taskNum);
            } catch (NumberFormatException e) {
                ui.showMessage("Please input an integer!");
            } catch (DukeException e) {
                ui.showMessage(e.getMessage());
            }
            break;
        case "unmark":
            try {
                if (command.equals("unmark")) {
                    throw new DukeException("Please type what you want to unmark!");
                }
                description = command.substring(firstWord.length() + 1);
                int taskNum = Integer.parseInt(description);
                return new UnmarkCommand(firstWord, taskNum);
            } catch (NumberFormatException e) {
                ui.showMessage("Please input an integer!");
            } catch (DukeException e) {
                ui.showMessage(e.getMessage());
            }
            break;
        case "deadline":
            try {
                description = command.substring(firstWord.length() + 1);
                description = description.split(" /by ")[0];
                String time = command.split(" /by ")[1];
                return new AddCommand(firstWord, description, time);
            } catch (ArrayIndexOutOfBoundsException e) {
                ui.showMessage("Eh? Did you mistype the format?");
            } catch (StringIndexOutOfBoundsException e) {
                System.out.println("Uh-oh! There's no deadline here!");
            }
            break;
        case "todo":
            try {
                if (command.equals("todo")) {
                    throw new DukeException("Uh-oh! There is nothing to do here!");
                }
                description = command.substring(firstWord.length() + 1);
                return new AddCommand(firstWord, description);
            } catch (DukeException e) {
                ui.showMessage(e.getMessage());
            }
            break;
        case "event":
            try {
                description = command.substring(firstWord.length() + 1);
                description = description.split(" /at ")[0];
                String time = command.split(" /at ")[1];
                return new AddCommand(firstWord, description, time);
            } catch (ArrayIndexOutOfBoundsException e) {
                ui.showMessage("Eh? Did you mistype the format?");
            } catch (StringIndexOutOfBoundsException e) {
                System.out.println("Uh-oh! There's no event here!");
            }
            break;
        case "find":
            try {
                if (command.equals("find")) {
                    throw new DukeException("Uh-oh! There is nothing to find here!");
                }
                description = command.substring(firstWord.length() + 1);
                return new FindCommand(firstWord, description);
            } catch (DukeException e) {
                ui.showMessage(e.getMessage());
            }
            break;
        default:
            // pass through
            break;
        }
        return new InvalidCommand("Please try again!");
    }

Example from src/main/java/duke/TaskList.java lines 109-152:

    public static ArrayList<Task> populateList(String fileName) throws IOException {
        ArrayList<Task> list = new ArrayList<Task>();
        BufferedReader file = new BufferedReader(new FileReader(fileName));
        String line = file.readLine();
        while (line != null) {
            boolean isMarked = String.valueOf(line.charAt(4)).equals("X");
            if (String.valueOf(line.charAt(1)).equals("T")) {
                Task task = new Todo(line.substring(7));
                if (isMarked) {
                    task = task.mark();
                }
                list.add(task);
            } else if (String.valueOf(line.charAt(1)).equals("D")) {
                String tempDescription = line.split("by: ")[0].substring(7);
                int tempDescLength = tempDescription.length();
                String description = tempDescription.substring(0, tempDescLength - 2);
                String tempTimeBy = line.split("by: ")[1];
                int endIdx = tempTimeBy.lastIndexOf(")");
                String timeBy = tempTimeBy.substring(0, endIdx);
                DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMM d yyyy");
                Task task = new Event(description, LocalDate.parse(timeBy, formatter).toString());
                if (isMarked) {
                    task = task.mark();
                }
                list.add(task);
            } else if (String.valueOf(line.charAt(1)).equals("E")) {
                String tempDescription = line.split("at: ")[0].substring(7);
                int tempDescLength = tempDescription.length();
                String description = tempDescription.substring(0, tempDescLength - 2);
                String tempTimeBy = line.split("at: ")[1];
                int endIdx = tempTimeBy.lastIndexOf(")");
                String timeBy = tempTimeBy.substring(0, endIdx);
                DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMM d yyyy");
                Task task = new Event(description, LocalDate.parse(timeBy, formatter).toString());
                if (isMarked) {
                    task = task.mark();
                }
                list.add(task);
            }
            line = file.readLine();
        }
        file.close();
        return list;
    }

Example from src/test/java/duke/DukeTest.java lines 10-42:

    public void parserTest() {
        Ui ui = new Ui();
        assertEquals(Parser.parse("list", ui).getFirstWord(), "list");
        assertEquals(Parser.parse("list test", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("todo", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("todo test", ui).getFirstWord(), "todo");
        assertEquals(Parser.parse("deadline", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("deadline /by", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("deadline test /by", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("deadline test/by 2020=-03-04", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("deadline test /by 2020-03-04", ui).getFirstWord(), "deadline");
        assertEquals(Parser.parse("deadline test /by 2020", ui).getFirstWord(), "deadline");
        assertEquals(Parser.parse("event", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("event /at", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("event test /at", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("event test/at 2020=-03-04", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("event test /at 2020-03-04", ui).getFirstWord(), "event");
        assertEquals(Parser.parse("event test /at 2020", ui).getFirstWord(), "event");
        assertEquals(Parser.parse("mark", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("mark test", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("unmark", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("unmark test", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("mark 1", ui).getFirstWord(), "mark");
        assertEquals(Parser.parse("unmark 1", ui).getFirstWord(), "unmark");
        assertEquals(Parser.parse("delete", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("delete test", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("delete 1", ui).getFirstWord(), "delete");
        assertEquals(Parser.parse("find", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("find test", ui).getFirstWord(), "find");
        assertEquals(Parser.parse("bye", ui).getFirstWord(), "bye");
        assertEquals(Parser.parse("bye test", ui).getFirstWord(), "Please try again!");
        assertEquals(Parser.parse("test", ui).getFirstWord(), "Please try again!");
    }

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

Example from src/main/java/duke/Deadline.java lines 12-17:

    /**
     * Construct an instance of a deadline, which is unmarked by default.
     *
     * @param description A string representing the task description.
     * @param by A String representing a time in the format of "yyyy-mm-dd"
     */

Example from src/main/java/duke/Deadline.java lines 23-29:

    /**
     * Construct an instance of a marked or unmarked deadline.
     *
     * @param description A string representing the task description.
     * @param by A String representing a time in the format of "yyyy-mm-dd".
     * @param isDone A boolean representing whether the task is marked as done.
     */

Example from src/main/java/duke/Event.java lines 12-17:

    /**
     * Construct an instance of an event, which is unmarked by default.
     *
     * @param description A string representing the task description.
     * @param at A String representing a time in the format of "yyyy-mm-dd"
     */

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 05af61e:

Merge remote-tracking branch 'origin/add-gradle-support' into branch-A-Gradle

• Longer than 72 characters

Suggestion: Follow the given conventions for Git commit messages for future commits (no need to modify past commit messages).

ℹ️ The bot account @nus-se-bot 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.