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

[nus-se-bot]

@ChinZJ 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

Example from src/main/java/botling/tasks/Task.java lines 12-12:

    private static final boolean TASK_NOT_DONE = false;

Suggestion: Follow the given naming convention for boolean variables/methods (e.g., use a boolean-sounding prefix).You may ignore the above if you think the name already follows the convention (the script can report false positives in some cases)

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/test/java/botling/TaskListWriterTest.java lines 67-107:

    public void filePresentTest() {
        String expectedMsg = "Attempting to retrieve history...\n"
                + "Data folder found!\n"
                + "History file found! Restoring data...\n";
        String expectedFileString =
                "event\n23 Jan 2025 0000\n24 Jan 2025 2359\n \ntrue\n05 Feb 2025 1116\n"
                + "todo\n \ntrue\n05 Feb 2025 1116\n"
                + "deadline\ntonight!\n \nfalse\n05 Feb 2025 1116";
        String expectedListString =
                " 1. [DATE] [E][X]   (from: 23 Jan 2025 0000 to: 24 Jan 2025 2359)\n"
                + " 2. [T][X]  \n"
                + " 3. [D][ ]   (by: tonight!)";

        LocalDateTime setTime = LocalDateTime.parse("05 Feb 2025 1116",
                DateTimeFormatter.ofPattern("dd MMM yyyy HHmm"));

        ToDo first = new ToDo(" ", true, LocalDateTime.parse("05 Feb 2025 1116",
                DateTimeFormatter.ofPattern("dd MMM yyyy HHmm")));
        Deadlines second = new Deadlines(" ", false, "tonight!",
                Optional.empty(), setTime);
        DateTimeFormatter format = DateTimeFormatter.ofPattern("yyyy-MM-dd HHmm");
        LocalDateTime startTime = LocalDateTime.parse("2025-01-23 0000", format);
        LocalDateTime endTime = LocalDateTime.parse("2025-01-24 2359", format);
        Events third = new Events(" ", true, "2025-01-23 0000", "2025-01-24 2359",
                Optional.of(startTime), Optional.of(endTime), setTime);

        TaskList expected = new TaskList();
        expected.add(first);
        expected.add(second);
        expected.add(third);

        TaskListWriter tester = new TaskListWriter();
        TaskList testList = new TaskList();
        String actual = tester.restore(testList);

        assertEquals(expectedMsg, actual);
        assertEquals(3, testList.size());
        assertEquals(expectedFileString, testList.fileString());
        assertEquals(expected.fileString(), testList.fileString());
        assertEquals(expectedListString, String.join("", testList.list()));
    }

Example from src/test/java/botling/messagegenerator/MsgGenTest.java lines 217-255:

    public void unknownTest() {
        resetVariables();
        String output = MsgGen.unknownCmd(cmdColor);

        assertEquals("Yikes!!! This command is still up for discussion.\n"
                + "Type 'help' for a list of commands and their syntax!", output);
        assertEquals(1, cmdColor.getMessages().length);
        assertEquals("Yikes!!! This command is still up for discussion.\n"
                + "Type 'help' for a list of commands and their syntax!",
                cmdColor.getMessages()[0]);
        assertEquals(ColorNames.COLOR_BLACK.getIndex(), cmdColor.getLines()[0]);

        cmdColor.reset();
        output = MsgGen.unknownSyntax("cmd", "syntax", cmdColor);

        assertEquals("Yikes!!! The format of cmd should be cmdsyntax", output);
        assertEquals(1, cmdColor.getMessages().length);
        assertEquals("Yikes!!! The format of cmd should be cmdsyntax",
                cmdColor.getMessages()[0]);
        assertEquals(ColorNames.COLOR_BLACK.getIndex(), cmdColor.getLines()[0]);

        cmdColor.reset();
        output = MsgGen.unknownCorrupt(cmdColor);

        assertEquals("Please enter 'y' for yes and 'n' for no.", output);
        assertEquals(1, cmdColor.getMessages().length);
        assertEquals("Please enter 'y' for yes and 'n' for no.", cmdColor.getMessages()[0]);
        assertEquals(ColorNames.COLOR_BLACK.getIndex(), cmdColor.getLines()[0]);

        cmdColor.reset();
        output = MsgGen.unknownDateTime(cmdColor);

        assertEquals("That date time looks awkwardly wrong...\n"
                + "I think this is suitable for peer help!", output);
        assertEquals(1, cmdColor.getMessages().length);
        assertEquals("That date time looks awkwardly wrong...\n"
                + "I think this is suitable for peer help!", cmdColor.getMessages()[0]);
        assertEquals(ColorNames.COLOR_BLACK.getIndex(), cmdColor.getLines()[0]);
    }

Example from src/test/java/botling/commands/CommandParserTest.java lines 325-378:

    public void eventTest() {
        resetVariables();

        // Standard input.
        String result = addTaskMessage(" [E][ ]   (from:   to:  )\n", 1);
        assertEquals(result, cmdParse.parse("event   /from   /to  ", tasks, cmdColor));

        // Nested event commandtypes.
        result = addTaskMessage(" [E][ ]   (from: event  to: /from asd /to asd )\n",
                2);
        assertEquals(result, cmdParse.parse("event   /from event  /to /from asd /to asd ",
                tasks, cmdColor));

        // Multiple /from /to commandtypes.
        result = addTaskMessage(" [E][ ] a (from: b /from c to: d /to e)\n", 3);
        assertEquals(result, cmdParse.parse("event a /from b /from c /to d /to e",
                tasks, cmdColor));

        result = addTaskMessage(" [E][ ] a (from: b to: c /from d /to e)\n", 4);
        assertEquals(result, cmdParse.parse("event a /from b /to c /from d /to e",
                tasks, cmdColor));

        result = addTaskMessage(" [E][ ] a (from: b to: c /to d /from e)\n", 5);
        assertEquals(result, cmdParse.parse("event a /from b /to c /to d /from e",
                tasks, cmdColor));

        result = addTaskMessage(" [E][ ] a /to b (from: c to: d /from e)\n", 6);
        assertEquals(result, cmdParse.parse("event a /to b /from c /to d /from e",
                tasks, cmdColor));

        result = addTaskMessage(" [E][ ] a /to b (from: c /from d to: e)\n", 7);
        assertEquals(result, cmdParse.parse("event a /to b /from c /from d /to e",
                tasks, cmdColor));

        // Doubles as invalid input.
        result = "Yikes!!! The format of event should be event <name> /from <start "
                + "(may be date)> /to <end (may be date)>.\n"
                + "<start> should be before or equal to <end> if dates are inputs.\n"
                + DATE_FORMAT_MESSAGE;
        assertEquals(result, cmdParse.parse("event a /to b /to c /from d /from e",
                tasks, cmdColor));

        // event with invalid start and end date
        assertEquals(result, cmdParse.parse(
                "event a /from 02 Jan 2024 0000 /to 02 Jan 2022 0000",
                tasks, cmdColor));

        // event with date.
        // Doubles to check that same date works, but not before.
        result = addTaskMessage(" [DATE] [E][ ]   "
                + "(from: 02 Jan 2024 0000 to: 02 Jan 2024 0000)\n", 8);
        assertEquals(result, cmdParse.parse("event   /from 2024-01-02 /to 2024-01-02",
                tasks, cmdColor));
    }

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/botling/tasks/Task.java lines 95-98:

    /**
     * Used by comparator class.
     * Returns date <code>Task</code> object was created
     */

Example from src/main/java/botling/tasks/Task.java lines 103-106:

    /**
     * Used by comparator class.
     * Returns start date of <code>Task</code>
     */

Example from src/main/java/botling/tasks/Task.java lines 111-113:

    /**
     * Used by comparator class.
     */

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 Messages

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.