Sharing iP code quality feedback [for @chengseong]

[nus-se-bot]

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 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/duke/Parser.java lines 50-116:

    public static Command parse(String fullCommand) throws IllegalCommandException, IllegalTaskException {
        String command = fullCommand.split(" ")[0];
        switch (command) {
        case "list":
            return new ListCommand();
        case "done":
            int toComplete = Integer.parseInt(fullCommand.split(" ")[1]) - 1;
            return new DoneCommand(toComplete);
        case "delete":
            int toDelete = Integer.parseInt(fullCommand.split(" ")[1]) - 1;
            return new DeleteCommand(toDelete);
        case "todo": {
            String task = fullCommand.replaceFirst("todo ", "");
            if (task.equals("todo")) {
                throw new IllegalTaskException("☹ OOPS!!! The description of a todo cannot be empty.");
            } else {
                return new AddCommand(new ToDo(task));
            }
        }
        case "deadline": {
            String[] taskDate = fullCommand.replaceFirst("deadline ", "").split("/by ");
            String task = taskDate[0];
            String date = taskDate[1];
            String[] splitDateTime = date.split(" ");
            String[] splitDate = splitDateTime[0].split("/");
            LocalDate localDate;
            if (splitDate[1].length() == 1) {
                localDate = LocalDate.parse(splitDate[2] + "-0" + splitDate[1] + "-" + splitDate[0]);
            } else if (splitDate[0].length() == 1) {
                localDate = LocalDate.parse(splitDate[2] + "-" + splitDate[1] + "-0" + splitDate[0]);
            } else {
                localDate = LocalDate.parse(splitDate[2] + "-" + splitDate[1] + "-" + splitDate[0]);
            }
            LocalTime localTime;
            localTime = LocalTime.parse(splitDateTime[1], DateTimeFormatter.ofPattern("HHmm"));
            return new AddCommand(new Deadline(task, localDate, localTime));
        }
        case "event": {
            String[] taskDate = fullCommand.replaceFirst("event ", "").split("/at ");
            String task = taskDate[0];
            String date = taskDate[1];
            String[] splitDateTime = date.split(" ");
            String[] splitDate = splitDateTime[0].split("/");
            LocalDate localDate;
            if (splitDate[1].length() == 1) {
                localDate = LocalDate.parse(splitDate[2] + "-0" + splitDate[1] + "-" + splitDate[0]);
            } else if (splitDate[0].length() == 1) {
                localDate = LocalDate.parse(splitDate[2] + "-" + splitDate[1] + "-0" + splitDate[0]);
            } else {
                localDate = LocalDate.parse(splitDate[2] + "-" + splitDate[1] + "-" + splitDate[0]);
            }
            LocalTime localTime;
            localTime = LocalTime.parse(splitDateTime[1], DateTimeFormatter.ofPattern("HHmm"));
            return new AddCommand(new Event(task, localDate, localTime));
        }
        case "find" : {
            String keyword = fullCommand.replaceFirst("find ", "");
            return new FindCommand(keyword);
        }
        case "bye": {
            return new ExitCommand();
        }
        default: {
            throw new IllegalCommandException("☹ OOPS!!! I'm sorry, but I don't know what that means :-(");
        }
        }
    }

Example from src/main/java/duke/Storage.java lines 41-80:

    public ArrayList<Task> load() throws FileNotFoundException {
        ArrayList<Task> list = new ArrayList<Task>();
        File f = new File(filePath);
        if (f.exists()) {
            Scanner s = new Scanner(f);
            while (s.hasNext()) {
                String toRead = s.nextLine();
                String[] strSplit = toRead.split(" \\| ");
                if (strSplit[0].equals("T")) {
                    ToDo toDo = new ToDo(strSplit[2]);
                    if (strSplit[1].equals("1")) {
                        toDo.complete();
                    }
                    list.add(toDo);
                } else if (strSplit[0].equals("D")) {
                    String[] dateTime = strSplit[3].split(", ");
                    LocalDate date = LocalDate.parse(dateTime[0], DateTimeFormatter.ofPattern("MMM dd yyyy"));
                    LocalTime time = LocalTime.parse(dateTime[1], DateTimeFormatter.ofPattern("hh:mm a"));
                    Deadline deadline = new Deadline(strSplit[2], date, time);
                    if (strSplit[1].equals("1")) {
                        deadline.complete();
                    }
                    list.add(deadline);
                } else if (strSplit[0].equals("E")) {
                    String[] dateTime = strSplit[3].split(", ");
                    LocalDate date = LocalDate.parse(dateTime[0], DateTimeFormatter.ofPattern("MMM dd yyyy"));
                    LocalTime time = LocalTime.parse(dateTime[1], DateTimeFormatter.ofPattern("hh:mm a"));
                    Event event = new Event(strSplit[2], date, time);
                    if (strSplit[1].equals("1")) {
                        event.complete();
                    }
                    list.add(event);
                }
            }
            s.close();
        } else {
            throw new FileNotFoundException();
        }
        return list;
    }

Suggestion: Consider applying SLAP (and other abstraction mechanisms) to shorten methods. You may ignore this suggestion if you think a longer method is justified in a particular case.

Aspect: Header Comments

Example from src/main/java/Duke.java lines 74-81:

    /**
     * Main method which starts the Duke bot.
     * @param args user input
     * @throws FileNotFoundException when filepath does not exist.
     * @throws FolderNotFoundException when filepath folder does not exist.
     * @throws IOException On input error.
     * @see IOException
     */

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 Message (Subject Only)

possible problems in commit 80af952:

Created GUI

• Not in imperative mood (?)

possible problems in commit d5e3f76:

Fixed gradle issue and shifted Java files into packages

• Not in imperative mood (?)

possible problems in commit bb2f790:

Fixed most checkstyle issues except NewlineAtEndofFile error

• Not in imperative mood (?)

Suggestion: Follow the given conventions for Git commit messages

ℹ️ The bot account @cs2103-bot used to post this issue is un-manned. Do not reply to this post (as those replies will not be read). Instead, contact [email protected] if you want to follow up on this post.