Skip to content

Sharing iP code quality feedback [for @brenda77777] #3

@soc-se-script

Description

@soc-se-script

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

No easy-to-detect issues 👍

Aspect: Method Length

Example from src/main/java/candy/Parser.java lines 14-117:

    public static ParsedCommand parse(String input) throws CandyException {
        input = input.trim();

        if (input.equals("bye")) {
            return new ParsedCommand(CommandType.BYE);
        }

        if (input.equals("list")) {
            return new ParsedCommand(CommandType.LIST);
        }

        if (input.equals("help")) {
            return new ParsedCommand(CommandType.HELP);
        }

        if (input.startsWith("mark")) {
            ParsedCommand cmd = new ParsedCommand(CommandType.MARK);
            cmd.index = parseIndex(input);
            return cmd;
        }

        if (input.startsWith("unmark")) {
            ParsedCommand cmd = new ParsedCommand(CommandType.UNMARK);
            cmd.index = parseIndex(input);
            return cmd;
        }

        if (input.startsWith("delete")) {
            ParsedCommand cmd = new ParsedCommand(CommandType.DELETE);
            cmd.index = parseIndex(input);
            return cmd;
        }

        if (input.startsWith("todo")) {
            String desc = input.substring(4).trim();
            if (desc.isEmpty()) {
                throw new CandyException("Please use format: todo <task>");
            }
            ParsedCommand cmd = new ParsedCommand(CommandType.TODO);
            cmd.description = desc;
            return cmd;
        }

        if (input.startsWith("deadline")) {
            String rest = input.substring(8).trim();
            String[] parts = rest.split(" /by ", 2);
            if (parts.length < 2) {
                throw new CandyException("Please use format: deadline <task> /by <yyyy-mm-dd>");
            }

            String desc = parts[0].trim();
            String by = parts[1].trim();
            if (desc.isEmpty() || by.isEmpty()) {
                throw new CandyException("Please use format: deadline <task> /by <yyyy-mm-dd>");
            }

            // validate date format
            parseDate(by);

            ParsedCommand cmd = new ParsedCommand(CommandType.DEADLINE);
            cmd.description = desc;
            cmd.byDate = by;
            return cmd;
        }

        if (input.startsWith("event")) {
            String rest = input.substring(5).trim();
            String[] parts = rest.split(" /from ", 2);
            if (parts.length < 2) {
                throw new CandyException("Please use format: event <task> /from <start> /to <end>");
            }

            String desc = parts[0].trim();
            String[] times = parts[1].split(" /to ", 2);
            if (times.length < 2) {
                throw new CandyException("Please use format: event <task> /from <start> /to <end>");
            }

            String from = times[0].trim();
            String to = times[1].trim();

            if (desc.isEmpty() || from.isEmpty() || to.isEmpty()) {
                throw new CandyException("Please use format: event <task> /from <start> /to <end>");
            }

            ParsedCommand cmd = new ParsedCommand(CommandType.EVENT);
            cmd.description = desc;
            cmd.fromTime = from;
            cmd.toTime = to;
            return cmd;
        }

        if (input.startsWith("find")) {
            String keyword = input.substring(4).trim();
            if (keyword.isEmpty()) {
                throw new CandyException("Please use format: find <keyword>");
            }
            ParsedCommand cmd = new ParsedCommand(CommandType.FIND);
            cmd.keyword = keyword;
            return cmd;
        }

        throw new CandyException("Unknown command. Type 'help' to see available commands");
    }

Example from src/main/java/candy/ParsedCommand.java lines 46-106:

    public String execute(TaskList tasks, Ui ui, Storage storage) throws CandyException {
        switch (type) {

        case LIST:
            return tasks.formatForDisplay();

        case TODO:
            tasks.add(new Todo(description));
            storage.saveLines(tasks.toLines());
            return "Added: " + description;

        case DEADLINE:
            LocalDate date = Parser.parseDate(byDate);
            tasks.add(new Deadline(description, date));
            storage.saveLines(tasks.toLines());
            return "Added deadline: " + description + " (by: " + date + ")";

        case EVENT:
            tasks.add(new Event(description, fromTime, toTime));
            storage.saveLines(tasks.toLines());
            return "Added event: " + description + " (from: " + fromTime + " to: " + toTime + ")";

        case MARK:
            tasks.mark(index);
            storage.saveLines(tasks.toLines());
            return "Marked task " + (index + 1);

        case UNMARK:
            tasks.unmark(index);
            storage.saveLines(tasks.toLines());
            return "Unmarked task " + (index + 1);

        case DELETE:
            tasks.remove(index); // if your method is delete(), change this line to tasks.delete(index)
            storage.saveLines(tasks.toLines());
            return "Deleted task " + (index + 1);

        case FIND:
            TaskList result = tasks.find(keyword);
            return result.formatForDisplay();

        case BYE:
            storage.saveLines(tasks.toLines()); // optional
            return "Bye. Hope to see you again soon!";

        case HELP:
            return "Available commands:\n"
                    + "list\n"
                    + "todo <task>\n"
                    + "deadline <task> /by <yyyy-mm-dd>\n"
                    + "event <task> /from <start> /to <end>\n"
                    + "mark <task number>\n"
                    + "unmark <task number>\n"
                    + "delete <task number>\n"
                    + "find <keyword>\n"
                    + "bye";

        default:
            throw new CandyException("Unknown command. Type 'help' to see available commands.");
        }
    }

Example from src/main/java/candy/Candy.java lines 48-120:

    public void run() {
        ui.showWelcome();

        while (true) {
            String input = ui.readCommand();

            try {
                ParsedCommand cmd = Parser.parse(input);

                switch (cmd.type) {
                case BYE:
                    saveAll();
                    ui.showMessage("Bye. Hope to see you again soon!");
                    return;

                case LIST:
                    ui.showList(tasks);
                    break;

                case TODO:
                    tasks.add(new Todo(cmd.description));
                    ui.showAdd(tasks.get(tasks.size() - 1), tasks.size());
                    saveAll();
                    break;

                case DEADLINE: {
                    LocalDate by = Parser.parseDate(cmd.fromTime);
                    tasks.add(new Deadline(cmd.description, by));
                    ui.showAdd(tasks.get(tasks.size() - 1), tasks.size());
                    saveAll();
                    break;
                }

                case EVENT:
                    tasks.add(new Event(cmd.description, cmd.fromTime, cmd.toTime));
                    ui.showAdd(tasks.get(tasks.size() - 1), tasks.size());
                    saveAll();
                    break;

                case MARK:
                    tasks.mark(cmd.index);
                    ui.showMark(tasks.get(cmd.index));
                    saveAll();
                    break;

                case UNMARK:
                    tasks.unmark(cmd.index);
                    ui.showUnmark(tasks.get(cmd.index));
                    saveAll();
                    break;

                case DELETE:
                    Task removed = tasks.remove(cmd.index);
                    ui.showDelete(removed, tasks.size());
                    saveAll();
                    break;

                case FIND:
                    TaskList matches = tasks.find(cmd.keyword);
                    ui.showFindResults(cmd.keyword, matches);
                    break;

                default:
                    throw new CandyException("OOPS!!! I'm sorry, but I don't know what that means :-(");
                }

            } catch (CandyException e) {
                ui.showError(e.getMessage());
            } catch (Exception e) {
                ui.showError("Something went wrong. Please try again.");
            }
        }
    }

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 677f3c3:


ParsedCommand: Replace arg1 and arg2 with explicit fields

ParsedCommand previously relied on generic positional arguments by arg1, arg2 to represent different meanings across commands.

This caused ambiguity, especially for the find command where arg1
represented a keyword instead of a task description.

Replace positional arguments with explicit fields such as keyword
and description to clarify command intent.

This improves readability and reduces the risk of future
bugs caused by unclear argument usage.

This improves readability and prevents future bugs caused by
overloading argument semantics.


  • body not wrapped at 72 characters: e.g., ParsedCommand previously relied on generic positional arguments by arg1, arg2 to represent different meanings across commands.

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

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


ℹ️ 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@nus.edu.sg if you want to follow up on this post.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions