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

[soc-se-bot-red]

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

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

Example from src/main/java/pixel/util/Parser.java lines 85-85:

                // return UserInterface.GOODBYE_MESSAGE;

Example from src/test/java/pixel/HandleTaskTest.java lines 24-24:

            //System.out.println(exception.toString());

Example from src/test/java/pixel/HandleTaskTest.java lines 35-35:

            // System.out.println(exception);

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/pixel/gui/MainWindow.java lines 92-130:

    private void readTasksFromFile(String filePath) {
        BufferedReader reader;
        try {
            reader = new BufferedReader(new FileReader(filePath));
            String line;
            line = reader.readLine();
            while (line != null) { // reader will continuously read the next line
                Task savedTask = lineToTask(line);
                Storage.INPUT_TASKS.add(savedTask);
                line = reader.readLine();
            }
            reader.close();
            Pixel.resetTaskCount(Storage.INPUT_TASKS.size());
            dialogContainer.getChildren().add(
                DialogBox.getDukeDialog(UserInterface.FILE_LOADED, pixelImage)
            );

        } catch (IOException e) {
            File dataFolder = new File("./data");
            dataFolder.mkdir();
            File tempFile = new File("./data/pixel.txt");
            try {
                tempFile.createNewFile();
            } catch (IOException ex) {
                dialogContainer.getChildren().add(
                    DialogBox.getDukeDialog("Unable to create new file to store tasks \n"
                        + "Please check your storage directory!", pixelImage)
                );
            }
            dialogContainer.getChildren().add(
                DialogBox.getDukeDialog(UserInterface.FILE_DOES_NOT_EXIST, pixelImage)
            );
        } catch (InvalidTextDataFormatException e) {
            dialogContainer.getChildren().add(
                DialogBox.getDukeDialog(e + "\n"
                    + UserInterface.FILE_TASKS_INVALID, pixelImage)
            );
        }
    }

Example from src/main/java/pixel/gui/MainWindow.java lines 141-191:

    private Task lineToTask(String lineFromDocument) throws InvalidTextDataFormatException {
        String[] componentsOfTask = lineFromDocument.strip().split(" ;;; ");
        String type = "";
        String status = "";
        String description = "";
        String commandWord = "";
        String due = "";

        try {
            type = componentsOfTask[0];
            status = componentsOfTask[1];
            description = componentsOfTask[2];

        } catch (IndexOutOfBoundsException e) {
            throw new InvalidTextDataFormatException("Task in the text file doesn't have a type, "
                + "status or description!");
        }

        if ((componentsOfTask.length == 4) && (!Objects.equals(componentsOfTask[3], " ;;;"))) {
            throw new InvalidTextDataFormatException("It seems like the fourth or fifth section of "
                + "one of the tasks in the text file is in bad format");
        }

        if (componentsOfTask.length == 5) {
            commandWord = handleCommandWord(componentsOfTask[3]);
            due = componentsOfTask[4];
        }

        if ((componentsOfTask.length != 5) && (componentsOfTask.length != 4)) {
            throw new InvalidTextDataFormatException(InvalidTextDataFormatException.BAD_TASK_FORMATTING);
        }

        switch (type.strip()) {
        case "T": {
            Task formattedTask = new ToDo(description, due, commandWord);
            return checkFormatOfTaskStatus(formattedTask, status);
        }
        case "D": {
            Task formattedTask = new Deadline(description, due, commandWord);
            return checkFormatOfTaskStatus(formattedTask, status);
        }

        case "E": {
            Task formattedTask = new Event(description, due, commandWord);
            return checkFormatOfTaskStatus(formattedTask, status);
        }

        default:
            throw new InvalidTextDataFormatException("Type of task in database is invalid!");
        }
    }

Example from src/main/java/pixel/util/Parser.java lines 78-210:

    public String parse(String userInput) {

        String strippedInput = userInput.strip();
        String lowerCaseStrippedInput = strippedInput.toLowerCase();

        try {
            if (lowerCaseStrippedInput.startsWith("bye")) {
                // return UserInterface.GOODBYE_MESSAGE;
                System.exit(0);

            } else if (lowerCaseStrippedInput.startsWith("todo ")) {
                return taskList.handleNewTask(strippedInput, TaskType.TODO);

            } else if (lowerCaseStrippedInput.startsWith("deadline ")) {
                return taskList.handleNewTask(strippedInput, TaskType.DEADLINE);

            } else if (lowerCaseStrippedInput.startsWith("event ")) {
                return taskList.handleNewTask(strippedInput, TaskType.EVENT);

            } else if (lowerCaseStrippedInput.startsWith("mark ")) {
                int indexToChange = getMarkOrUnmarkIndex(strippedInput, Marking.MARK);

                if ((indexToChange < 1) || (indexToChange > 100)) {
                    throw new IndexOutOfBoundsException("Only index 1 to 100 are supported");
                }

                Storage.INPUT_TASKS.get(indexToChange - 1).markAsDone();
                Storage.resetFile(this.filePath);
                Storage.appendAllTasksToFile(this.filePath);

                return (" Nice! I've marked this task as done: \n"
                    + Storage.INPUT_TASKS.get(indexToChange - 1) + "\n"
                    + UserInterface.AFTER_VALID_INPUT);

            } else if (lowerCaseStrippedInput.startsWith("unmark ")) {
                int indexToChange = getMarkOrUnmarkIndex(strippedInput, Marking.UNMARK);

                if ((indexToChange < 1) || (indexToChange > 100)) {
                    throw new IndexOutOfBoundsException("Only index 1 to 100 are supported");
                }

                Storage.INPUT_TASKS.get(indexToChange - 1).markAsNotDone();
                Storage.resetFile(this.filePath);
                Storage.appendAllTasksToFile(this.filePath);

                return ("OK, I've marked this task as not done yet: \n"
                    + Storage.INPUT_TASKS.get(indexToChange - 1) + "\n"
                    + UserInterface.AFTER_VALID_INPUT);

            } else if (lowerCaseStrippedInput.equals("list")) {
                String listOfTasks = TaskList.listTasks();
                return listOfTasks + UserInterface.AFTER_VALID_INPUT;

            } else if (lowerCaseStrippedInput.startsWith("delete ")) {
                String output = Storage.deleteEntry(strippedInput, filePath);
                return output + "\n" + UserInterface.AFTER_VALID_INPUT;

            } else if (lowerCaseStrippedInput.startsWith("find ")) {
                ArrayList<Task> results = Storage.findEntry(strippedInput);
                String findResults = TaskList.listFindResults(results);
                return findResults + UserInterface.AFTER_VALID_INPUT;

            } else {
                throw new IncorrectFormatException("Input should be a task or command!"); // programme breaks
            }

        } catch (IndexOutOfBoundsException e) {
            return (e.getMessage() + "\n"
                + "Make sure you enter valid information after \"/at\" or \"/by\" \n"
                + "And check if the total number of tasks does not exceed 100 \n"
                + UserInterface.AFTER_INVALID_INPUT + "\n"
                + UserInterface.PROMPT_MESSAGE);

        } catch (StackOverflowError e) {
            return ("caught Stack Overflow Error \n"
                + UserInterface.AFTER_INVALID_INPUT + "\n"
                + UserInterface.PROMPT_MESSAGE);

        } catch (NullPointerException e) {
            return ("caught Null pointer exception \n"
                + UserInterface.AFTER_INVALID_INPUT + "\n"
                + UserInterface.PROMPT_MESSAGE);

        } catch (IncorrectFormatException e) {
            return (
                e + "\n"
                + UserInterface.AFTER_INVALID_INPUT + "\n"
                + UserInterface.PROMPT_MESSAGE);

        } catch (IOException e) {
            if (e instanceof FileNotFoundException) {
                File dataFolder = new File("./data");
                dataFolder.mkdir();
                File tempFile = new File("./data/pixel.txt");
                try {
                    tempFile.createNewFile();
                } catch (IOException ex) {
                    return ("Unable to create new file to store tasks \n"
                        + "Please check your storage directory!");
                }
                return ("Caught FileNotFound exception! \n"
                    + "New file is created for you \n"
                    + UserInterface.PROMPT_MESSAGE);
            } else {
                return ("Caught IO exception! \n"
                    + UserInterface.PROMPT_MESSAGE);
            }

        } catch (DuplicateEntryException e) {
            return (e + "\n"
                + "You may want to add a different task :) \n"
                + UserInterface.AFTER_INVALID_INPUT + "\n"
                + UserInterface.PROMPT_MESSAGE);

        } catch (DateTimeParseException e) {
            return ("Please ensure that your date & time input are in yyyy-MM-dd(SPACE)HHmm(24h) format \n"
                + UserInterface.PROMPT_MESSAGE);

        } catch (ParseException e) {
            return (e.getMessage() + "\n"
                + UserInterface.AFTER_INVALID_INPUT + "\n"
                + UserInterface.PROMPT_MESSAGE);

        } catch (Exception e) {
            return ("Some other error occurred, please check you input :) \n"
                + UserInterface.AFTER_INVALID_INPUT + "\n"
                + UserInterface.PROMPT_MESSAGE);
        }

        // Should not reach here
        throw new IncorrectFormatException("Oops! Tell the developer "
            + "that there's something wrong with the parse method");
    }

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

Example from src/main/java/pixel/Pixel.java lines 66-69:

    /**
     * Obsolete method to run Pixel.
     * Previously used for CLI.
     */

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)

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.