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

[soc-se-bot-red]

@qilstiano 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/einstein/Einstein.java lines 18-18:

    private boolean welcomeShown = false;

Example from src/main/java/einstein/ui/Ui.java lines 101-101:

        boolean found = 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/main/java/einstein/parser/Parser.java lines 36-76:

    public static Command parse(String fullCommand) throws EinsteinException {
        assert fullCommand != null : "Full command cannot be null";
        assert !fullCommand.trim().isEmpty() : "Full command cannot be empty";
        Command command;

        if (fullCommand.equalsIgnoreCase("bye")) {
            command = new ExitCommand();
        } else if (fullCommand.equalsIgnoreCase("list")) {
            command = new ListCommand();
        } else if (fullCommand.startsWith("list ")) {
            command = new ListByDateCommand(fullCommand);
        } else if (fullCommand.startsWith("mark ")) {
            command = new MarkCommand(fullCommand);
        } else if (fullCommand.startsWith("unmark ")) {
            command = new UnmarkCommand(fullCommand);
        } else if (fullCommand.startsWith("todo ")) {
            command = parseTodoCommand(fullCommand);
        } else if (fullCommand.startsWith("deadline ")) {
            command = new AddDeadlineCommand(fullCommand);
        } else if (fullCommand.startsWith("event ")) {
            command = new AddEventCommand(fullCommand);
        } else if (fullCommand.startsWith("delete ")) {
            command = new DeleteCommand(fullCommand);
        } else if (fullCommand.equalsIgnoreCase("help")) {
            command = new HelpCommand();
        } else if (fullCommand.startsWith("find ")) {
            command = new FindCommand(fullCommand);
        } else if (fullCommand.startsWith("tag ")) {
            command = new AddTagCommand(fullCommand);
        } else if (fullCommand.startsWith("untag ")) {
            command = new RemoveTagCommand(fullCommand);
        } else if (fullCommand.startsWith("priority ")) {
            command = new SetPriorityCommand(fullCommand);
        } else {
            throw new EinsteinException("ARGH! I do not understand you, which is weird, "
                    + "\nbecause I usually understand most things. Invalid command!");
        }

        assert command != null : "Parsed command should not be null";
        return command;
    }

Example from src/main/java/einstein/command/HelpCommand.java lines 23-61:

    public String execute(TaskList tasks, Ui ui, Storage storage) throws EinsteinException {
        assert tasks != null : "TaskList cannot be null";
        assert ui != null : "UI cannot be null";
        assert storage != null : "Storage cannot be null";
        // Default help prompt listed below
        StringBuilder output = new StringBuilder();
        output.append("Ja, here are the commands I understand:\n\n");
        output.append("1. todo <description> - Add a todo task.\n");
        output.append("   Example: todo read book\n\n");
        output.append("2. deadline <description> /by <date> - Add a deadline task.\n");
        output.append("   Example: deadline return book /by 2/12/2019 1800\n\n");
        output.append("3. event <description> /from <start> /to <end> - Add an event task.\n");
        output.append("   Example: event project meeting /from 2/12/2019 1400 /to 2/12/2019 1600\n\n");
        output.append("4. list - List all tasks.\n");
        output.append("   Example: list\n\n");
        output.append("5. list <date> - List tasks occurring on a specific date (format: yyyy-MM-dd).\n");
        output.append("   Example: list 2019-12-02\n\n");
        output.append("6. find <task name> - List tasks matching the searched name\n");
        output.append("   Example: list read books\n\n");
        output.append("7. mark <task number> - Mark a task as done.\n");
        output.append("   Example: mark 1\n\n");
        output.append("8. unmark <task number> - Mark a task as not done.\n");
        output.append("   Example: unmark 1\n\n");
        output.append("9. delete <task number> - Delete a task.\n");
        output.append("   Example: delete 1\n\n");
        output.append("10. tag <task number> <tag> - Add a tag to a task.\n");
        output.append("   Example: tag 1 fun\n\n");
        output.append("11. untag <task number> <tag> - Remove a tag from a task.\n");
        output.append("   Example: untag 1 fun\n\n");
        output.append("12. priority <task number> <uber_high|high|medium|low> "
                + "\nSet a priority to a task.\n");
        output.append("   Example: priority 1 low\n\n");
        output.append("13. help - Display this help message.\n");
        output.append("   Example: help\n\n");
        output.append("14. bye - Exit the program.\n");
        output.append("   Example: bye");

        return output.toString();
    }

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/einstein/gui/Launcher.java lines 11-16:

    /**
     * The main entry point for the Einstein GUI application.
     * This method launches the JavaFX application by calling the Main class.
     *
     * @param args Command-line arguments passed to the application (not used).
     */

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

possible problems in commit 775ff4b:

Add AI.md to document AI-assisted contributions

Add AI.md to outline the specific ways AI was used in the development of the Einstein chatbot.
This includes AI-assisted documentation, README formatting, debugging support, and assertion writing.
The document also clarifies human oversight in AI-generated content.

This improves project transparency regarding AI usage.

• body not wrapped at 72 characters: e.g., Add AI.md to outline the specific ways AI was used in the development of the Einstein chatbot.

possible problems in commit 6203ef8:

Add Javadoc comments to test classes for better documentation

The test classes (ParserTest, StorageTest, TaskListTest) currently lack
Javadoc comments, making it harder to understand the purpose and
functionality of each test case.

Adding Javadoc comments improves the readability and maintainability of
the test suite by providing clear documentation for each class and test
method.

Let's:
- Add class-level Javadoc comments to ParserTest, StorageTest, and
  TaskListTest, explaining the purpose of each class.
- Add method-level Javadoc comments to each test method, describing what
  the test is checking, what it's verifying, and any exceptions that
  might be thrown.

These comments will:
- Make it easier for developers to understand the intent and behavior
  of each test.
- Serve as a valuable reference for future maintenance and modifications
  of the test suite.
- Improve the overall quality and clarity of the codebase.

This enhancement helps ensure the tests are well-documented, contributing to
the long-term maintainability of the project.

• body not wrapped at 72 characters: e.g., This enhancement helps ensure the tests are well-documented, contributing to

possible problems in commit f5c2e6f:

Add assertions to AddTagCommand for input and state validation

The AddTagCommand class is currently missing checks for input validity
and object state, potentially leading to unexpected behavior
or errors during runtime.

Adding assertions improves the class's reliability by validating command
parameters and ensuring the TaskList, Ui, and Storage objects are in a
consistent state.

Let's:
- Add assertions to the constructor to validate fullCommand, taskIndex, and tag.
  Ensure they are non-null and non-empty where appropriate and taskIndex is
  valid.
- Add assertions to the execute method to validate TaskList, Ui, and Storage.
  Ensure they are not null, and validate the task list and task at the given
  index are also not null.
- Validate the result string returned after adding the tag to ensure there are no unexpected issues

These validations:
- Catch invalid inputs early, preventing potential runtime errors.
- Ensure essential dependencies (TaskList, Ui, Storage) are correctly
  provided.
- Confirm the task at the given index exists before attempting to add the tag.
- Validate the result string to confirm to user it worked

This proactive approach enhances the class's robustness and makes debugging
easier by clearly identifying the source of errors.

• body not wrapped at 72 characters: e.g., - Add assertions to the constructor to validate fullCommand, taskIndex, and tag.

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.