Sharing iP code quality feedback [for @tyanhan]

[nus-se-bot]

@tyanhan 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/saitama/parser/Parser.java lines 148-197:

    private static Command prepareAdd(String[] splitCommand, ArrayList<Tag> tags) throws
            InvalidFormatException, EmptyDescriptionException, InvalidCommandException, InvalidDateTimeException {
        if (splitCommand.length < 2 || splitCommand[1].equals("")) {
            throw new EmptyDescriptionException();
        }
        String taskType = splitCommand[0];
        String taskArguments = splitCommand[1];

        RecurFrequency recurFrequency = null;
        for (Tag tag : tags) {
            if (tag instanceof RecurFrequency) {
                @SuppressWarnings("unchecked")
                RecurFrequency rf = (RecurFrequency) tag;
                recurFrequency = rf;
                break;
            }
        }

        assert taskType.equals("TODO") || taskType.equals("DEADLINE") || taskType.equals("EVENT")
                : "Parser detected invalid task type to add!";

        switch (taskType) {
        case "TODO":
            Task newTask = new ToDo(taskArguments, recurFrequency);
            return new AddCommand(newTask);
        case "DEADLINE":
            String[] taskArgumentsList = taskArguments.split(" /by ", 2);
            if (taskArgumentsList.length < 2) {
                throw new InvalidFormatException("You need to specify "
                        + "the deadline!\n deadline <task name> /by <dd/mm/yyyy hh:mm>");
            }
            String taskDescription = taskArgumentsList[0];
            LocalDateTime deadline = processDateTime(taskArgumentsList[1]);
            newTask = new Deadline(taskDescription, deadline, recurFrequency);
            return new AddCommand(newTask);
        case "EVENT":
            taskArgumentsList = taskArguments.split(" /at ", 2);
            if (taskArgumentsList.length < 2) {
                throw new InvalidFormatException("You need to specify event location!\n"
                        + "event <task name> /at <location>");
            }
            taskDescription = taskArgumentsList[0];
            String location = taskArgumentsList[1];
            newTask = new Event(taskDescription, location, recurFrequency);
            return new AddCommand(newTask);
        default:
            //Should never happen since we asserted that the task type is valid
            throw new InvalidCommandException();
        }
    }

Example from src/main/java/saitama/storage/Storage.java lines 44-127:

    public ArrayList<Task> load() {
        assert filePath.endsWith(".txt") : "File path needs to be a .txt file";

        ArrayList<Task> taskList = new ArrayList<>();
        File f = new File(filePath);

        try {
            Scanner sc = new Scanner(f);
            while (sc.hasNext()) {
                //split each line in text file into [task type, isDone, task description]
                String[] data = sc.nextLine().split(" ", 5);

                if (data.length < 5) {
                    throw new FileCorruptException();
                }

                //assign isDone
                boolean isDone;
                switch(data[1]) {
                case "0":
                    isDone = false;
                    break;
                case "1":
                    isDone = true;
                    break;
                default:
                    throw new FileCorruptException();
                }

                //assign recursiveTag
                RecurFrequency recurFrequency = RecurFrequency.get(data[2]);
                //assign last reset date
                LocalDate lastResetDate = LocalDate.parse(data[3]);

                //add task to array list
                String taskType = data[0];
                String taskArguments = data[4];
                switch(taskType) {
                case "T":
                    taskList.add(new ToDo(taskArguments, isDone, recurFrequency, lastResetDate));
                    break;
                case "D":
                    String[] splitTaskArguments = taskArguments.split(" /by ", 2);
                    if (splitTaskArguments.length < 2) {
                        throw new FileCorruptException();
                    }

                    String description = splitTaskArguments[0];
                    String deadlineString = splitTaskArguments[1];

                    try {
                        LocalDateTime deadline = Parser.processDateTime(deadlineString);
                        Task newTask = new Deadline(description,
                                deadline, isDone, recurFrequency, lastResetDate);
                        taskList.add(newTask);
                    } catch (InvalidFormatException | InvalidDateTimeException e) {
                        throw new FileCorruptException();
                    }
                    break;
                case "E":
                    splitTaskArguments = taskArguments.split(" /at ", 2);
                    if (splitTaskArguments.length < 2) {
                        throw new FileCorruptException();
                    }

                    description = splitTaskArguments[0];
                    String location = splitTaskArguments[1];
                    Task newTask = new Event(description,
                            location, isDone, recurFrequency, lastResetDate);
                    taskList.add(newTask);
                    break;
                default:
                    throw new FileCorruptException();
                }
            }
            return taskList;
        } catch (FileCorruptException e) {
            System.out.println(e.getMessage());
            return new ArrayList<>();
        } catch (FileNotFoundException e) {
            System.out.printf("Filepath %s not found. Creating new list...\n", filePath);
            return new ArrayList<>();
        }
    }

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

Example from src/main/java/saitama/parser/Parser.java lines 236-241:

    /**
     * A helper function to test if a String can be converted to integer.
     *
     * @param string The string to be converted to integer.
     * @return Whether the string can be converted to integer.
     */

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 👍

ℹ️ 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.