Skip to content

Sharing iP code quality feedback [for @jinnhl] #1

@nus-se-bot

Description

@nus-se-bot

@jinnhl 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/jeff/parser/Parser.java lines 30-130:

    public static Command parse(String fullCommand) throws JeffException {
        String[] splitCommand = fullCommand.split(" ", 2);
        String keyword = splitCommand[0];

        // To prevent NullPointerException for when body does not exist.
        String body = null;
        int len = splitCommand.length;
        if (len > 1) {
            body = splitCommand[1];
        }

        // Perform the correct instructions according to the keyword.
        switch(keyword) {
        case ("bye"):
            return new ByeCommand();
        case ("list") :
            return new ListCommand();
        case ("mark"):
            // When no index is given.
            if (len == 1) {
                throw new JeffException(" ☹ OOPS!!! Please tell me the task's"
                        + " index number so that I can mark it as done.");
            }
            return new MarkCommand(body);
        case ("unmark"):
            // When no index is given.
            if (len == 1) {
                throw new JeffException(" ☹ OOPS!!! Please tell me the task's"
                        + " index number so that I can mark it as not done.");
            }
            return new UnmarkCommand(body);
        case ("todo"):
            // When no description is given.
            if (len == 1) {
                throw new JeffException(" ☹ OOPS!!! The description of a todo cannot be empty.");
            }
            return new TodoCommand(body);
        case ("deadline"):
            // When no description is given.
            if (len == 1) {
                throw new JeffException(" ☹ OOPS!!! The description of a deadline cannot be empty.");
            }

            String[] splitBody = body.split(" /by ", 2);
            String description = splitBody[0];

            // To prevent NullPointerException for when body does not exist.
            String dateInfo = null;
            int bodyLength = splitBody.length;
            if (bodyLength > 1) {
                dateInfo = splitBody[1];
            }

            // When no description or dateInfo are given.
            if (description.equals("") || description.equals(" ")) {
                throw new JeffException(" ☹ OOPS!!! The description of a deadline cannot be empty.");
            } else if (bodyLength == 1 || dateInfo.equals("") || dateInfo.equals(" ")) {
                throw new JeffException(" ☹ OOPS!!! Please input a due date for this task");
            } else {
                return new DeadlineCommand(description, dateInfo);
            }
        case ("event"):
            // When no description is given.
            if (len == 1) {
                throw new JeffException(" ☹ OOPS!!! The description of a event cannot be empty.");
            }
            splitBody = body.split(" /at ", 2);
            description = splitBody[0];

            // To prevent NullPointerException for when body does not exist.
            dateInfo = null;
            bodyLength = splitBody.length;
            if (bodyLength > 1) {
                dateInfo = splitBody[1];
            }

            // When no description or dateInfo are given.
            if (description.equals("") || description.equals(" ")) {
                throw new JeffException(" ☹ OOPS!!! The description of a event cannot be empty.");
            } else if (bodyLength == 1 || dateInfo.equals("") || dateInfo.equals(" ")) {
                throw new JeffException(" ☹ OOPS!!! Please input a due date for this task");
            } else {
                return new EventCommand(description, dateInfo);
            }
        case ("delete"):
            // When no description is given.
            if (len == 1) {
                throw new JeffException(" ☹ OOPS!!! Please tell me the task's"
                        + " index number so that I can delete it from the list.");
            }
            return new DeleteCommand(body);
        case ("find"):
            if (len == 1) {
                throw new JeffException(" ☹ OOPS!!! Please tell me the keyword"
                        + " so that I know what you are looking for.");
            }
            return new FindCommand(body);
        default:
            return new HelpCommand();
        }
    }

Example from src/main/java/jeff/storage/Storage.java lines 42-103:

    public TaskList load() throws JeffException {

        File f = new File(filePath);
        TaskList tasks = new TaskList();

        // When the file or directory does not exist, create it.
        Scanner sc;
        try {
            sc = new Scanner(f);
        } catch (FileNotFoundException e) {
            File parent = f.getParentFile();
            if (!parent.exists()) {
                parent.mkdir();
            }
            return tasks;
        }

        // Look through every line on the external file and add to TaskList.
        while (sc.hasNext()) {
            String input = sc.nextLine();
            try {
                String[] inputLine = input.split("\\| ", 4);
                String type = inputLine[0];
                String done = inputLine[1];
                String name = inputLine[2];
                String time;

                // New task from the current line
                Task curr;
                // Check what type of task is this.
                switch (type) {
                case "T ":
                    curr = new Todo(name);
                    break;
                case "D ":
                    time = inputLine[3];
                    curr = new Deadline(name.substring(0, name.length() - 1), time);
                    break;
                case "E ":
                    time = inputLine[3];
                    curr = new Event(name.substring(0, name.length() - 1), time);
                    break;
                default:
                    curr = null;
                }
                // Check if the task is marked as done.
                switch (done) {
                case "1 ":
                    curr.setMark();
                case "0 ":
                    break;
                }
                // After all the necessary information, add it into the TaskList.
                tasks.add(curr);
            } catch (DateTimeParseException e) {
                throw new JeffException("Invalid date time format at: " + input
                        + "Please refer to readme.txt for the available formats");
            }
        }
        sc.close();
        return tasks;
    }

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: Class size

No easy-to-detect issues 👍

Aspect: Header Comments

No easy-to-detect issues 👍

Aspect: Recent Git Commit Message (Subject Only)

No easy-to-detect issues 👍

ℹ️ The bot account @nus-se-bot 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.

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