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

[nus-se-script]

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

Example from src/test/java/Judy/TodoTest.java lines 1-1:

package Judy;

Example from src/test/java/Judy/TaskListTest.java lines 1-1:

package Judy;

Example from src/test/java/Judy/TaskTest.java lines 1-1:

package Judy;

Suggestion: Follow the package naming convention specified by the coding standard.

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/Judy/ui/Parser.java lines 37-116:

    public static Command parse(String input) throws JudyException {
        assert input != null : "Input command should not be null";

        if (input.equals("list")) {
            return new ListCommand();

        } else if (input.startsWith("find")) {
            String[] parts = input.split(" ");
            assert parts.length > 0 : "Split parts should not be empty";
            if (parts.length > 1) {
                return new FindCommand(parts[1].trim());
            } else {
                throw new JudyException("Invalid find command. Usage: find <keyword>");
            }

        } else if (input.startsWith("mark") || input.startsWith("unmark")) {
            String[] parts = input.split(" ");
            assert parts.length > 0 : "Split parts should not be empty";
            if (parts.length > 1) {
                int number = Integer.parseInt(parts[1]);
                boolean isMark = input.startsWith("mark");
                return new MarkCommand(number, isMark);
            } else {
                throw new JudyException("Invalid mark command. Usage: mark <task number>");
            }

        } else if (input.startsWith("todo")) {
            String description;
            if (input.length() < 5) {
                description = "";
            } else {
                description = input.substring(5);
            }
            return new AddCommand(TaskType.TODO, description, null, null, null);
        } else if (input.startsWith("deadline")) {
            String[] parts = input.split("/by");
            assert parts.length > 0 : "Split parts should not be empty";
            String description;
            if (parts.length == 2) {
                if (parts[0].trim().length() < 9) {
                    description = "";
                } else {
                    description = parts[0].trim().substring(9);
                }
                String deadline = parseDateTime(parts[1].trim());
                return new AddCommand(TaskType.DEADLINE, description, deadline, null, null);
            } else {
                throw new JudyException("Invalid deadline format. Use: deadline <description> /by <time>");
            }

        } else if (input.startsWith("event")) {
            String[] parts = input.split(" /from | /to ");
            assert parts.length > 0 : "Split parts should not be empty";
            String description;
            if (parts.length == 3) {
                if (parts[0].trim().length() < 6) {
                    description = "";
                } else {
                    description = parts[0].trim().substring(6);
                }
                String start = parseDateTime(parts[1].trim());
                String end = parseDateTime(parts[2].trim());
                return new AddCommand(TaskType.EVENT, description, null, start, end);
            } else {
                throw new JudyException("Invalid event format. Use: event <description> /from <start> /to <end>");
            }
        } else if (input.startsWith("delete")) {
            String[] parts = input.split(" ");
            assert parts.length > 0 : "Split parts should not be empty";
            if (parts.length == 2) {
                int number = Integer.parseInt(parts[1]);
                assert number > 0 : "Task number must be positive";
                return new DeleteCommand(number);
            } else {
                throw new JudyException("Invalid delete format. Use: delete <index>");
            }
        } else {
            throw new JudyException("Unknown command. Please try again with a valid command.");
        }
    }

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

No easy-to-detect issues 👍

Aspect: Recent Git Commit Messages

possible problems in commit 7716da5:

Refactor: Fix typos in variable names for clarity

Some variable names contained typos, which could lead to confusion and
reduce code readability.

Fix this by correcting the typos to ensure consistency and maintainability.

• body not wrapped at 72 characters: e.g., Fix this by correcting the typos to ensure consistency and maintainability.

possible problems in commit 9aafbbd:

Bug fix: Prevent StringIndexOutOfBoundsException for empty todo descriptions

Previously, entering a `todo` command without a description caused a
`StringIndexOutOfBoundsException`, as the code attempted to access
characters beyond the valid index range.

An unhandled exception can cause unexpected crashes and poor user
experience. Ensuring proper validation prevents such issues.

Let's fix this by:
* Adding a check to detect empty `todo` descriptions before processing
* Displaying a meaningful error message instead of throwing an exception

This improves stability and ensures the application handles invalid
input gracefully.

• Longer than 72 characters

possible problems in commit 0ee4c87:

Docs: Update README.md with correct release link  Previously, the README.md contained an incorrect or outdated release link,  which could mislead users when downloading the application.  Providing the correct release link ensures that users can easily access  the latest version without confusion.  Let's update the README.md to include the correct release link, improving  the accuracy and usability of the documentation.

• Longer than 72 characters
• 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

• /Users/damithch/Repos/folder_for_temp_data/AY2425S2/cs2103/./ip/ip/ki1r0/bin/Deadline.class

• /Users/damithch/Repos/folder_for_temp_data/AY2425S2/cs2103/./ip/ip/ki1r0/bin/InputHandler.class

• /Users/damithch/Repos/folder_for_temp_data/AY2425S2/cs2103/./ip/ip/ki1r0/bin/Event.class

Suggestion: Avoid committing binary files (e.g., *.class, *.jar, *.exe) or third-party library files in to the repo.

---

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