Sharing iP code quality feedback [for @m0destly]

[nus-se-bot]

@m0destly 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/darwin/TaskList.java lines 114-199:

    public String add(String[] inputs) throws DarwinException {
        if (inputs[TASK_TYPE_DESCRIPTION].equals("todo") || inputs[TASK_TYPE_DESCRIPTION].startsWith("todo ")) {
            try {
                String description = inputs[TASK_TYPE_DESCRIPTION].substring(TODO_DESCRIPTION_INDEX).trim();
                // No description
                if (description.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_DESCRIPTION_TODO.message());
                }
                tasks.add(new Todo(description));
            } catch (IndexOutOfBoundsException e) {
                throw new DarwinException(ErrorMessage.MISSING_DESCRIPTION_TODO.message());
            }
        } else if (inputs[TASK_TYPE_DESCRIPTION].equals("deadline") || inputs[TASK_TYPE_DESCRIPTION].startsWith("deadline ")) {
            try {
                String description = inputs[TASK_TYPE_DESCRIPTION].substring(DEADLINE_DESCRIPTION_INDEX).trim();
                // No description
                if (description.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_DESCRIPTION_DEADLINE.message());
                }
                String deadline = inputs[DEADLINE_DATE];
                // Does not begin with by
                if (!(deadline.equals("by") || deadline.startsWith("by "))) {
                    throw new DarwinException(ErrorMessage.WRONG_DEADLINE.message());
                }
                // No deadline
                deadline = deadline.substring(DEADLINE_DATE_INDEX).trim();
                if (deadline.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_DEADLINE.message());
                }
                LocalDate date = LocalDate.parse(deadline);
                tasks.add(new Deadline(description, date));
            } catch (ArrayIndexOutOfBoundsException | DateTimeParseException e) {
                throw new DarwinException(ErrorMessage.WRONG_DEADLINE.message());
            } catch (IndexOutOfBoundsException e) {
                throw new DarwinException(ErrorMessage.MISSING_DESCRIPTION_DEADLINE.message());
            }
        } else if (inputs[TASK_TYPE_DESCRIPTION].equals("event") || inputs[TASK_TYPE_DESCRIPTION].startsWith("event ")) {
            try {
                String description = inputs[TASK_TYPE_DESCRIPTION].substring(EVENT_DESCRIPTION_INDEX).trim();
                // No description
                if (description.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_DESCRIPTION_EVENT.message());
                }
                String from = inputs[EVENT_FROM];
                // Does not begin with from
                if (!(from.equals("from") || from.startsWith("from "))) {
                    throw new DarwinException(ErrorMessage.WRONG_EVENT.message());
                }
                from = from.substring(EVENT_FROM_INDEX).trim();
                // No start time
                if (from.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_START.message());
                }
                LocalDate fromDate = LocalDate.parse(from);
                String to = inputs[EVENT_TO];
                // Does not begin with to
                if (!(to.equals("to") || to.startsWith("to "))) {
                    throw new DarwinException(ErrorMessage.WRONG_EVENT.message());
                }
                to = to.substring(EVENT_TO_INDEX).trim();
                // No end time
                if (to.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_END.message());
                }
                LocalDate toDate = LocalDate.parse(to);
                tasks.add(new Event(description, fromDate, toDate));
            } catch (ArrayIndexOutOfBoundsException | DateTimeParseException e) {
                throw new DarwinException(ErrorMessage.WRONG_EVENT.message());
            } catch (IndexOutOfBoundsException e) {
                throw new DarwinException(ErrorMessage.MISSING_DESCRIPTION_EVENT.message());
            }
        } else {
            throw new DarwinException(ErrorMessage.UNKNOWN.message());
        }

        // Retrieves current task from back
        Task task = tasks.get(tasks.size() - 1);
        String output = "Got it. I've added this task:\n  " + task + "\nNow you have ";
        if (tasks.size() == 1) {
            output = output.concat("1 task");
        } else {
            output = output.concat(tasks.size() + " tasks");
        }
        output = output.concat(" in the list.");
        return output;
    }

Example from src/main/java/darwin/Parser.java lines 21-74:

    public static String parse(String command, TaskList taskList) throws DarwinException {
        if (command.equals("bye")) {
            return Ui.showExit();
        } else if (command.equals("list")) {
            return taskList.list();
        } else if (command.equals("help")) {
            return Ui.showHelp();
        } else if (command.equals("mark") || command.startsWith("mark ")) {
            try {
                String index = command.substring(Parser.MARK_INDEX).trim();
                // No index
                if (index.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_INDEX_MARK.message());
                }
                int taskNumber = Integer.parseInt(index) - 1;
                return taskList.mark(taskNumber);
            } catch (NumberFormatException e) {
                throw new DarwinException(ErrorMessage.NOT_NUMBER.message());
            }
        } else if (command.equals("unmark") || command.startsWith("unmark ")) {
            try {
                String index = command.substring(Parser.UNMARK_INDEX).trim();
                // No index
                if (index.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_INDEX_UNMARK.message());
                }
                int taskNumber = Integer.parseInt(index) - 1;
                return taskList.unmark(taskNumber);
            } catch (NumberFormatException e) {
                throw new DarwinException(ErrorMessage.NOT_NUMBER.message());
            }
        } else if (command.equals("delete") || command.startsWith("delete ")) {
            try {
                String index = command.substring(Parser.DELETE_INDEX).trim();
                // No index
                if (index.isEmpty()) {
                    throw new DarwinException(ErrorMessage.MISSING_INDEX_DELETE.message());
                }
                int taskNumber = Integer.parseInt(index) - 1;
                return taskList.delete(taskNumber);
            } catch (NumberFormatException e) {
                throw new DarwinException(ErrorMessage.NOT_NUMBER.message());
            }
        } else if (command.equals("find") || command.startsWith("find ")) {
            String keyword = command.substring(Parser.FIND_INDEX).trim();
            if (keyword.isEmpty()) {
                throw new DarwinException(ErrorMessage.MISSING_KEYWORD.message());
            }
            return taskList.find(keyword);
        } else {
            String[] inputs = command.split(" /");
            return taskList.add(inputs);
        }
    }

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 dca6048:

Merge branch 'branch-C-Help'

* branch-C-Help:
  Add help command to access list of all commands, fix tasks not saved on closing GUI window

• body not wrapped at 72 characters: e.g., Add help command to access list of all commands, fix tasks not saved on closing GUI window

possible problems in commit aa6f410:

Add help command to access list of all commands, fix tasks not saved on closing GUI window

• Longer than 72 characters

possible problems in commit e68eba9:

Adjust to adhere to coding standards

Replaced magic numbers with named constants, replaced complicated expressions with more readable code, and happy path is more prominent.

• body not wrapped at 72 characters: e.g., Replaced magic numbers with named constants, replaced complicated expressions with more readable code, and happy path is more prominent.

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.