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

[nus-se-bot]

@jhwan0707 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/johan/ui/Ui.java lines 122-122:

        boolean found = false;

Example from src/main/java/johan/command/MarkCommand.java lines 13-13:

    private final boolean markAsDone;

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

Example from src/test/java/johan/JohanTest.java lines 1-1:

//package johan;

Example from src/test/java/johan/JohanTest.java lines 3-3:

//import org.junit.jupiter.api.Test;

Example from src/test/java/johan/JohanTest.java lines 4-4:

//import static org.junit.jupiter.api.Assertions.assertEquals;

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/johan/parser/Parser.java lines 26-75:

    public Command parse(String input) throws Exception {
        if (input.equals("bye")) {
            return new ExitCommand();
        } else if (input.equals("list")) {
            return new ListCommand();
        } else if (input.startsWith("mark ")) {
            int id = Integer.parseInt(input.substring(5)) - 1;
            return new MarkCommand(id, true);
        } else if (input.startsWith("unmark ")) {
            int id = Integer.parseInt(input.substring(7)) - 1;
            return new MarkCommand(id, false);
        } else if (input.startsWith("todo ")) {
            String desc = input.substring(5).trim();
            return new TodoCommand(desc);
        } else if (input.startsWith("deadline ")) {
            int byIndex = input.indexOf("/by");
            if (byIndex == -1) {
                throw new IllegalArgumentException("Please specify a deadline with /by.");
            }
            String desc = input.substring(9, byIndex).trim();
            String by = input.substring(byIndex + 4).trim();
            return new DeadlineCommand(desc, by);
        } else if (input.startsWith("event ")) {
            int fromIndex = input.indexOf("/from");
            int toIndex = input.indexOf("/to");
            if (fromIndex == -1 || toIndex == -1) {
                throw new IllegalArgumentException("Please specify /from and /to.");
            }
            String desc = input.substring(6, fromIndex).trim();
            String from = input.substring(fromIndex + 6, toIndex).trim();
            String to = input.substring(toIndex + 4).trim();
            return new EventCommand(desc, from, to);
        } else if (input.startsWith("delete ")) {
            int id = Integer.parseInt(input.substring(7)) - 1;
            return new DeleteCommand(id);
        } else if (input.startsWith("on ")) {
            String dateStr = input.substring(3).trim();
            return new OnDateCommand(dateStr);
        } else if (input.startsWith("find ")) {
            String keyword = input.substring(5).trim();
            if (keyword.isEmpty()) {
                throw new IllegalArgumentException("Please specify a keyword.");
            }
            return new FindCommand(keyword);
        } else if (input.startsWith("sort")) {
            return new SortCommand();
        } else {
            throw new IllegalArgumentException("I'm sorry, but I don't know what that means :-(");
        }
    }

Example from src/main/java/johan/storage/Storage.java lines 59-116:

    public ArrayList<Task> loadTasks() {
        ArrayList<Task> tasks = new ArrayList<>();
        File file = new File(filePath);

        if (!file.exists()) {
            System.out.println("No saved tasks found. Starting fresh.");
            return tasks;
        }

        try (BufferedReader reader = new BufferedReader(new FileReader(file))) {
            String line;
            // DateTimeFormatter inputFormatter = DateTimeFormatter.ofPattern("d/M/yyyy HHmm");
            while ((line = reader.readLine()) != null) {
                String[] parts = line.split(" \\| ");
                if (parts.length < 3) {
                    continue;
                }

                String type = parts[0];
                boolean isDone = parts[1].equals("1");
                String description = parts[2];

                Task task;
                if (type.equals("T")) {
                    task = new Todo(description);
                } else if (type.equals("D") && parts.length == 4) {
                    try {
                        // LocalDate deadline = LocalDate.parse(parts[3], inputFormatter);
                        task = new Deadline(description, parts[3]);
                    } catch (Exception e) {
                        System.out.println("Error parsing deadline for task: " + description + ". Skipping...");
                        continue;
                    }
                } else if (type.equals("E") && parts.length == 5) {
                    try {
                        // LocalDate from = LocalDate.parse(parts[3], inputFormatter);
                        // LocalDate to = LocalDate.parse(parts[4], inputFormatter);
                        task = new Event(description, parts[3], parts[4]);
                    } catch (Exception e) {
                        System.out.println("Error parsing deadline for task: " + description + ". Skipping...");
                        continue;
                    }
                } else {
                    continue; // Ignore invalid entries
                }

                if (isDone) {
                    task.markAsDone();
                }

                tasks.add(task);
            }
        } catch (IOException e) {
            System.out.println("Error loading tasks: " + e.getMessage());
        }

        return tasks;
    }

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/johan/Johan.java lines 101-105:

    /**
     * Main entry point for the application.
     *
     * @param args Command-line arguments (unused)
     */

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 7985e9f:

Add user guide to docs/README.md for Johan

Johan lacks a user guide, leaving users without clear instructions.

A guide enhances usability by detailing all key features, meeting the
A-UserGuide requirement for a product website.

Let's:
- Create docs/README.md with Johan Chatbot guide and Ui.png
- Cover adding deadlines, managing, sorting, searching, and exiting
- Prepare for GitHub Pages under /docs

This uses GFMD with a template structure, keeping it concise and user-friendly.

• body not wrapped at 72 characters: e.g., This uses GFMD with a template structure, keeping it concise and user-friendly.

possible problems in commit 8481439:

Added Ui.png under docs folder

• Not in imperative mood (?)

possible problems in commit a91ca69:

Updating Task.java

• Not in imperative mood (?)

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.