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

[nus-se-script]

@RobotWizzard 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/test/java/bob/ParserTest.java lines 15-15:

        // Tokenizing an empty string returns {"CMD": ""}

Example from src/test/java/bob/ParserTest.java lines 18-18:

        // Tokenizing a command without arguments returns {"CMD": <command>}

Example from src/test/java/bob/ParserTest.java lines 21-21:

        // Tokenizing a command with unnamed argument returns {"CMD": <cmd>, "", <arg>}

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/test/java/bob/ParserTest.java lines 14-55:

    public void tokenize_success() {
        // Tokenizing an empty string returns {"CMD": ""}
        assertEquals(Map.of("CMD", ""), Parser.tokenize(""));

        // Tokenizing a command without arguments returns {"CMD": <command>}
        assertEquals(Map.of("CMD", "list"), Parser.tokenize("list"));

        // Tokenizing a command with unnamed argument returns {"CMD": <cmd>, "", <arg>}
        assertEquals(
                Map.of("CMD", "mark", "", "2"),
                Parser.tokenize("mark 2"));
        assertEquals(
                Map.of("CMD", "todo", "", "A todo!"),
                Parser.tokenize("todo A todo!"));

        // Tokenizing a command with unnamed and named arguments returns {"CMD": <cmd>, "": <arg>, <name>: <arg>, ...}
        assertEquals(
                Map.of("CMD", "deadline", "", "A deadline!", "by", "tmr"),
                Parser.tokenize("deadline A deadline! /by tmr"));
        assertEquals(
                Map.of("CMD", "event",
                        "", "An event :O",
                        "from", "now",
                        "to", "tomorrow"),
                Parser.tokenize("event An event :O /from now /to tomorrow"));

        // Tokenizing a command with unnamed arguments only returns {"CMD": <cmd>, <name>: <arg>, ...}
        assertEquals(
                Map.of("CMD", "blah",  "dummy", "dummy value"),
                Parser.tokenize("blah /dummy dummy value"));
        assertEquals(
                Map.of("CMD", "please",  "do", "this", "and", "that"),
                Parser.tokenize("please /do this /and that"));

        // Tokenizing a named argument without value returns {..., <name>: ""}
        assertEquals(
                Map.of("CMD", "test",  "dummy", ""),
                Parser.tokenize("test /dummy"));
        assertEquals(
                Map.of("CMD", "test",  "dummy", "", "another", ""),
                Parser.tokenize("test /dummy /another"));
    }

Example from src/test/java/bob/ParserTest.java lines 58-119:

    public void parse_validCommand_success() {
        Parser parser = new Parser();

        // deadline
        assertEquals(
                new DeadlineCommand(Map.of(
                        "CMD", "deadline",
                        "", "test description",
                        "by", "tmr")),
                parser.parse("deadline test description /by tmr"));

        // delete
        assertEquals(
                new DeleteCommand(Map.of(
                        "CMD", "delete",
                        "", "5")),
                parser.parse("delete 5"));

        // event
        assertEquals(
                new EventCommand(Map.of(
                        "CMD", "event",
                        "", "test description",
                        "from", "now",
                        "to", "tmr")),
                parser.parse("event test description /from now /to tmr"));

        // exit
        assertEquals(new ExitCommand(Map.of("CMD", "bye")), parser.parse("bye"));

        // find
        assertEquals(
                new FindCommand(Map.of("CMD", "find", "", "book")),
                parser.parse("find book"));

        // list
        assertEquals(new ListCommand(Map.of("CMD", "list")), parser.parse("list"));

        // mark
        assertEquals(
                new MarkCommand(Map.of(
                        "CMD", "mark",
                        "", "5")),
                parser.parse("mark 5"));

        // reset
        assertEquals(new ResetCommand(Map.of("CMD", "reset")), parser.parse("reset"));

        // todo
        assertEquals(
                new TodoCommand(Map.of(
                        "CMD", "todo",
                        "", "test description")),
                parser.parse("todo test description"));

        // unmark
        assertEquals(
                new UnmarkCommand(Map.of(
                        "CMD", "unmark",
                        "", "5")),
                parser.parse("unmark 5"));
    }

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/bob/Bob.java lines 61-63:

    /**
     * Cleanup function when this Bob instance exits.
     */

Example from src/main/java/bob/TaskList.java lines 120-126:

    /**
     * Remove a specific tag name from the task at index {@code i} in this list.
     *
     * @param i the index of the task to be untagged
     * @param tagName the name of the tag to be removed
     * @return true if the specified task was previously tagged with the given tag name
     */

Example from src/main/java/bob/util/FormattedString.java lines 40-45:

    /**
     * Append the given string to the back of this {@code FormattedString}.
     *
     * @param str the string to append
     * @return a reference to this object
     */

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 3927cd5:

change numbered lists to a code block in the user guide.

Numbered lists do not show up as expected in github pages.

Let's use code blocks in place of numbered lists instead.

• Subject line should not end with a period

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 👍

---

❗ 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.