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

[nus-se-script]

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

No easy-to-detect issues 👍

Aspect: Brace Style

Example from src/main/java/duke/command/DeleteCommand.java lines 19-20:

        }
        catch (IndexOutOfBoundsException e) {

Example from src/main/java/duke/command/MarkCommand.java lines 19-20:

        }
        catch (IndexOutOfBoundsException e) {

Example from src/main/java/duke/command/UnmarkCommand.java lines 19-20:

        }
        catch (IndexOutOfBoundsException e) {

Suggestion: As specified by the coding standard, use egyptian style braces.

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/Parser.java lines 130-164:

    public Command parse(String input) {
        Command cmd;
        if (checkList(input)) {
            cmd = new ListCommand();
        } else if (checkFind(input)) {
            String word = input.split(" ")[1];
            cmd = new FindCommand(word);
        } else if (checkMark(input)) {
            int num = Integer.parseInt(input.split(" ")[1]);
            cmd = new MarkCommand(num);
        } else if (checkUnmark(input)) {
            int num = Integer.parseInt(input.split(" ")[1]);
            cmd = new UnmarkCommand(num);
        } else if (checkDelete(input)) {
            int num = Integer.parseInt(input.split(" ")[1]);
            cmd = new DeleteCommand(num);
        } else if (checkTask(input)) {
            if (!checkValidTask(input)) {
                cmd = new InvalidCommand("Please enter task description!");
            } else if (checkDeadline(input) && !isValidDeadline(input)) {
                cmd = new InvalidCommand("Format of deadline should be: " +
                        "deadline <description> /by: <dd/mm/yyyy hhmm");
            } else if (checkEvent(input) && !isValidEvent(input)) {
                cmd = new InvalidCommand("Format of event should be: " +
                        "event <description> /from: dd/mm/yyyy hhmm /to dd/mm/yyyy hhmm");
            } else {
                cmd = new AddCommand(input);
            }
        } else if (!isValidCommand(input)) {
            cmd = new InvalidCommand();
        } else {
            cmd = new ByeCommand();
        }
        return cmd;
    }

Example from src/main/java/duke/parser/Parser.java lines 166-213:

    public Task parseTask(String input) {
        Task newTask;
        String[] inputs = input.split(" ");
        String type = inputs[0];
        switch(type) {
            case "todo": {
                String name = input.split(" ", 2)[1];
                newTask = new ToDo(name);
                break;
            }
            case "deadline": {
                String[] nameAndDeadline = input.split(" ", 2)[1].split(" /by ");
                String name = nameAndDeadline[0];
                String[] deadline = nameAndDeadline[1].split(" ");
                String date = formatDate(deadline[0].split("/"));
                String remarks = " | " + nameAndDeadline[1];
                if (deadline.length == 1) {
                    newTask = new Deadline(name, date, remarks);
                } else {
                    String time = formatTime(deadline[1].split(""));
                    newTask = new Deadline(name, date, time, remarks);
                }
                break;
            }
            case "event": {
                String[] nameAndStart = input.split(" ", 2)[1].split(" /from ");
                String name = nameAndStart[0];
                String[] startAndEnd = nameAndStart[1].split(" /to ");
                String[] start = startAndEnd[0].split(" ");
                String startDate = formatDate(start[0].split("/"));
                String[] end = startAndEnd[1].split(" ");
                String endDate = formatDate(end[0].split("/"));
                String remarks = " | " + startAndEnd[0] + " | " + startAndEnd[1];
                if (start.length > 1) {
                    String startTime = formatTime(start[1].split(""));
                    String endTime = formatTime(end[1].split(""));
                    newTask = new Event(name, startDate, endDate, startTime, endTime, remarks);
                } else {
                    newTask = new Event(name, startDate, endDate, remarks);
                }
                break;
            }
            default: {
                throw new DukeException("☹ OOPS!!! I'm sorry, but I don't know what that means :-(");
            }
        }
        return newTask;
    }

Example from src/main/java/duke/storage/Storage.java lines 32-103:

    public static TaskList loadData() throws IOException {
        Parser logic = new Parser();
        TaskList list = new TaskList();
        if (!dataFolder.exists()) {
            System.out.println("The data folder is not found, creating new data folder");
            dataFolder.mkdir();
            System.out.println("Folder created successfully");
            File f = new File(dataFolder, "duke.txt");
            try {
                f.createNewFile();
                System.out.printf("file created successfully\n");
            } catch (IOException e) {
                throw new DukeException("Error creating file: " + e.getMessage());
            }
        } else if(!data.exists()) {
            System.out.println("file does not exist, creating new file");
            File f = new File(dataFolder, "duke.txt");
            try {
                f.createNewFile();
                System.out.printf("file create successfully");
            } catch (IOException e) {
                throw new DukeException("Error creating file: " + e.getMessage());
            }
        } else {
            List<String> tmp = Files.readAllLines(Paths.get(path), StandardCharsets.UTF_8);
            Iterator<String> itr = tmp.iterator();
            Task newTask;
            while (itr.hasNext()) {
                String curr = itr.next();
                String[] currLine = curr.split(" \\| ");
                String type = currLine[0];
                String description = currLine[2];
                switch (type) {
                    case "D": {
                        String[] deadline = currLine[3].split(" ");
                        String date = logic.formatDate(deadline[0].split("/"));
                        String remarks = " | " + currLine[3];
                        if (deadline.length == 1) {
                            newTask = new Deadline(description, date, remarks);
                        } else {
                            String time = logic.formatTime(deadline[1].split(""));
                            newTask = new Deadline(description, date, time, remarks);
                        }
                        break;
                    }
                    case "E": {
                        String[] start = currLine[3].split(" ");
                        String[] end = currLine[4].split(" ");
                        String startDate = logic.formatDate(start[0].split("/"));
                        String endDate = logic.formatDate(end[0].split("/"));
                        String remarks = " | " + currLine[3] + " | " + currLine[4];
                        if (start.length > 1) {
                            String startTime = logic.formatTime(start[1].split(""));
                            String endTime = logic.formatTime(end[1].split(""));
                            newTask = new Event(description, startDate, endDate, startTime, endTime, remarks);
                        } else {
                            newTask = new Event(description, startDate, endDate, remarks);
                        }
                        break;
                    }
                    default: {
                        newTask = new ToDo(description);
                    }
                }
                if (currLine[1].equals("X")) {
                    newTask.mark();
                }
                list.add(newTask);
            }
        }
        return list;
    }

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

No easy-to-detect issues 👍

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.