Sharing iP code quality feedback [for @aditig0305]

[soc-se-bot-red]

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

Example from src/main/java/max/task/Deadline.java lines 45-45:

        //return String.format("[D]" + super.toString() + " (by: %s)", date == null ? by : date);

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/max/main/Parser.java lines 56-101:

    public boolean parseText(String text) throws MaxException {

        assert taskList != null : "Task list is not initialized.";
        assert ui != null : "UI is not initialized.";
        assert storage != null : "Storage is not initialized.";

        try {
            if (text.equals("bye")) {
                ui.printBye();
                return true;
            } else if (text.equals("hi") || text.equals("hello")) {
                ui.printHello();
            } else if (text.equals("list")) {
                handleList();
            } else if (text.equals("help")) {
                handleHelp();
            } else if (text.startsWith("mark")) {
                int index = Integer.parseInt(text.replace("mark ", "")) - 1;
                handleMark(index);
            } else if (text.startsWith("unmark")) {
                int index = Integer.parseInt(text.replace("unmark ", "")) - 1;
                handleUnmark(index);
            } else if (text.startsWith("deadline")) {
                handleDeadline(text);
            } else if (text.startsWith("todo")) {
                handleTodo(text);
            } else if (text.startsWith("event")) {
                handleEvent(text);
            } else if (text.startsWith("delete")) {
                int index = Integer.parseInt(text.replace("delete ", "")) - 1;
                handleDelete(index);
            } else if (text.startsWith("find")) {
                String toFind = text.replaceFirst("find", "").trim();
                handleFind(toFind);
            } else {
                throw new MaxException("What does that mean?:( Begin with todo, event, or deadline.");
            }
            this.storage.saveTasks(taskList.getTasks());
        } catch (MaxException e) {
            ui.printLine();
            ui.printMessage(e.getMessage());
        }

        return false;

    }

Example from src/main/java/max/main/Ui.java lines 181-224:

    public void printHelp() {
        printLine();
        printToMax("Welcome to Max Help Page!");
        printToMax("Here are some commands you can use:");
        printToMax("");

        printToMax("'todo <description>'");
        printToMax("    Adds a new todo item.");
        printToMax("");

        printToMax("'deadline <description> /by <date>'");
        printToMax("    Adds a new deadline item.");
        printToMax("");

        printToMax("'event <description> /from <start> /to <end>'");
        printToMax("    Adds a new event item.");
        printToMax("");

        printToMax("'mark <index>'");
        printToMax("    Marks the task at the given index as done.");
        printToMax("");

        printToMax("'unmark <index>'");
        printToMax("    Marks the task at the given index as not done.");
        printToMax("");

        printToMax("'delete <index>'");
        printToMax("    Deletes the task at the given index.");
        printToMax("");

        printToMax("'list'");
        printToMax("    Lists all tasks.");
        printToMax("");

        printToMax("'find <keyword>'");
        printToMax("    Finds tasks containing the specified keyword.");
        printToMax("");

        printToMax("'help'");
        printToMax("    Shows this help page.");
        printToMax("");

        printLine();
    }

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 Message

possible problems in commit 60e4396:

 Add help page with command descriptions

- Implemented a help page for Max to assist users with available commands.
- Updated the print formatting to clearly list commands and their descriptions.

This update enhances user experience by providing clear guidance on using Max effectively.

• body not wrapped at 72 characters: e.g., - Implemented a help page for Max to assist users with available commands.

possible problems in commit 003f498:

Refactor long methods and remove unused imports

Current Situation:
- Several methods in the codebase, particularly in the Parser and task handling, were long and doing multiple things at once.
- This made the methods difficult to read, understand, and maintain.
- Some imports were present in the codebase but were no longer being used, adding unnecessary clutter.

Why It Needs to Change:
- Long methods often violate the Single Responsibility Principle (SRP) and can make the code harder to test.
- Refactoring them into smaller, more modular methods improves readability, maintainability, and allows for easier debugging and testing.
- Unused imports increase clutter and may lead to confusion or performance issues, making the codebase less clean.

What is Being Done:
- Refactored larger methods such as `parseText`, `handleMark`, `handleUnmark`, and others into smaller helper methods like `handleList`, `handleDelete`, etc.
- Extracted distinct logical units from long methods into smaller methods with clear, focused responsibilities.
- Updated corresponding Javadocs to document these changes.
- Removed all unused imports to declutter the code and avoid any unnecessary dependencies.

Why It Is Done This Way:
- Breaking down the logic into smaller, well-named methods makes it easier to follow and modify the code without introducing errors.
- This refactoring adheres to the principle of writing cleaner, more modular code.
- Removing unused imports enhances code cleanliness and efficiency, contributing to better overall code hygiene.

Any Other Relevant Info:
- No changes in functionality; the refactoring is purely for improving code structure and readability.

• body not wrapped at 72 characters: e.g., - Several methods in the codebase, particularly in the Parser and task handling, were long and doing multiple things at once.

possible problems in commit 0adcb57:

Add assertions to enforce input validation and task constraints across classes

- **Parser class:**
  - Asserted that the `taskList`, `ui`, and `storage` objects are not null in the constructor to
	ensure these components are initialised before use.
  - Added assertions to validate non-null and non-empty strings in methods like `handleDeadline`,
	`handleTodo`, and `handleEvent` to prevent invalid task descriptions.
  - Asserted that the task index used in methods like `mark`, `unmark`, and `delete` is within
	valid bounds to avoid IndexOutOfBoundsException.
- **Ui class:**
  - Asserted that the task list size is non-negative in methods like `printTaskTypeAdded` and `printDeleteTask` to prevent invalid states.
  - Ensured tasks are not null before printing them in methods like `printMarkDone` and `printMarkNotDone`.
  - Added checks to ensure that the task list passed to the `list` method is not null, preventing null pointer dereferencing.

These assertions improve robustness by validating key preconditions, catching errors early, and preventing common runtime exceptions such as null pointers or invalid indices.

• Longer than 72 characters
• body not wrapped at 72 characters: e.g., - Asserted that thetaskList, ui, and storage objects are not null in the constructor to

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 👍

---

ℹ️ 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.