Sharing iP code quality feedback [for @timothysashimi]

[soc-se-bot-blue]

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

Example from src/main/java/duke/Toothless.java lines 58-59:

        }
        catch (FileNotFoundException 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

Example from src/main/java/duke/Parser.java lines 26-26:

    //String response;

Example from src/main/java/duke/Parser.java lines 29-29:

        //this.response = "";

Example from src/main/java/duke/Parser.java lines 119-119:

                //TaskList temp = new TaskList();

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/duke/Parser.java lines 40-145:

    public Pair<TaskList, String> parse(TaskList tasksList, String input) throws InvalidInstructionException {
        String output = "";
        if (!input.toLowerCase().equals("bye")) {

            if (input.equals("list")) {
                return new Pair<TaskList, String>(tasksList, tasksList.toString());

            } else if (input.toLowerCase().startsWith("todo")) {
                try {
                    if (input.split(" ").length == 1) {
                        throw new MissingToDoNameException("Please provide the description of the todo task :) Eg. 'Todo Chores'");
                    } else {
                        String name = input.substring(5);
                        String response = tasksList.add(new ToDo(name, false, "T"));
                        output += this.line;
                        output += (response + "\n");
                        output += this.line;
                    }
                } catch (MissingToDoNameException err) {
                    output += this.line;
                    output += (err.getMessage());
                    output += this.line;
                }


            } else if (input.toLowerCase().startsWith("deadline")) {
                int endChar = input.indexOf("/");
                int startChar = 9;
                String name = input.substring(9, endChar);
                String deadline = input.substring(endChar + 4);
                LocalDate d = DateTimeParser.stringToDT(deadline);
                String response = tasksList.add(new Deadline(name, d, false, "D"));
                output += this.line;
                output += (response);
                output += this.line;

            } else if (input.toLowerCase().startsWith("event")) {
                int endChar = input.indexOf("/");
                int endChar2 = input.indexOf("/", endChar + 1);
                int startChar = 6;
                String name = input.substring(6, endChar);
                String startTime = input.substring(endChar + 5, endChar2);
                String endTime = input.substring(endChar2 + 3);
                LocalDate start = DateTimeParser.stringToDT(startTime);
                LocalDate end = DateTimeParser.stringToDT(endTime);
                String response = tasksList.add(new Event(name, start, end, false, "E"));
                output += this.line;
                output += (response);
                output += this.line;

            } else if (input.toLowerCase().startsWith("unmark")) {
                assert (input.split(" ").length != 1) : "Please provide a task to unmark :)";

                int index = Integer.parseInt(input.substring(7));
                String response = tasksList.unmark(index);
                output += (response);


            } else if (input.toLowerCase().startsWith("mark")) {
                try {
                    if (input.split(" ").length == 1) {
                        throw new MissingTaskToMarkException("Please provide a task to mark :)");
                    } else {
                        int index = Integer.parseInt(input.substring(5));
                        String response = tasksList.mark(index);
                        output += (response);
                    }
                } catch (MissingTaskToMarkException err) {
                    output += (err.getMessage());
                }


            } else if (input.toLowerCase().startsWith("delete")) {
                int index = Integer.parseInt(input.substring(7));
                String response = tasksList.delete(index);
                output += (response);
            //task name can partially contain keyword
            } else if (input.toLowerCase().startsWith("find")) {
                String keyword = input.split(" ")[1];
                //TaskList temp = new TaskList();
                TaskList temp = new TaskList(tasksList.getTasksList().stream()
                        .filter(t -> t.getTaskName().contains(keyword))
                        .collect(Collectors
                                .toCollection(ArrayList::new)));
                output += this.line;
                output += "Here are the matching tasks in your list:\n";
                output += temp.toString();
                output += ("\n" + this.line);
            } else {
                output += ("Try entering a valid instruction! Eg. 'Todo Chores' or 'Mark 2'\n");
            }

        } else {
            output = "Bye! Hope to see you again soon!";
            Storage storage = new Storage(tasksList);
            try {
                storage.store();
            } catch (IOException e) {
                System.err.println("Error writing to the file: " + e.getMessage());
                e.printStackTrace();
            }
        }


        return new Pair<TaskList, String>(tasksList, output);
    }

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

    public void store() throws IOException{
        String filePathOld = "data/toothless.txt";
        File oldFile = new File(filePathOld);
        String filePathNew = "data/temp.txt";
        File newFile = new File(filePathNew);
        try {
            boolean fileCreated = newFile.createNewFile();

        } catch (IOException e) {
            System.err.println("Error creating the file: " + e.getMessage());
            e.printStackTrace();
        }
        FileWriter fw = new FileWriter(filePathNew);
        for (Task t : taskList.getTasksList()) {
            String textToAdd = "";
            String taskType = t.getTaskType();
            if (taskType.equals("T")) {
                String name = t.getTaskName();
                String isDone = "0";
                if (t.getIsDone()) {
                    isDone = "1";
                }
                textToAdd += (taskType + " | " + isDone + " | " + name + "\n");
                fw.write(textToAdd);
                //System.out.println("1");
            } else if (taskType.equals("D")) {
                String name = t.getTaskName();
                String deadline = DateTimeParser.dtToString(t.getDeadline());
                String isDone = "0";
                if (t.getIsDone()) {
                    isDone = "1";
                }
                textToAdd += (taskType + " | " + isDone + " | " + name + " | " + deadline + "\n");
                fw.write(textToAdd);
            } else if (taskType.equals("E")) {
                String name = t.getTaskName();
                String start = DateTimeParser.dtToString(t.getStart());
                String end = DateTimeParser.dtToString(t.getEnd());
                String isDone = "0";
                if (t.getIsDone()) {
                    isDone = "1";
                }
                textToAdd += (taskType + " | " + isDone + " | " + name + " | " + " from " + start + " to " + end + "\n");
                fw.write(textToAdd);
            }
        }
        //System.out.println("1");
        fw.close();

        Path source = Paths.get("data/temp.txt");
        Path destination = Paths.get("data/toothless.txt");
        //oldFile.delete();
        Files.copy(source, destination, StandardCopyOption.REPLACE_EXISTING);

    }

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/duke/Toothless.java lines 37-39:

    /**
     * Method to instantiate the Toothless class. Toothless is instantiated when Launcher is run, which will run the main class and call this method.
     */

Example from src/main/java/duke/Ui.java lines 47-49:

    /**
     * For formatting purposes
     */

Example from src/main/java/duke/tasks/Deadline.java lines 22-24:

    /**
     * To mark the task as done using the superclass's mark method.
     */

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

possible problems in commit 3318479:

Added more javadoc comments

• Not in imperative mood (?)

possible problems in commit 6d72c32:

Improved find method

Task name just has to match keyword partially for it to be recognised.

• Not in imperative mood (?)

possible problems in commit d1eaba7:

Added streams in this commit

Previously, the find method uses a for loop to iterate through the tasklist. Hence a stream is introduced for efficiency.

• Not in imperative mood (?)
• body not wrapped at 72 characters: e.g., Previously, the find method uses a for loop to iterate through the tasklist. Hence a stream is introduced for efficiency.

Suggestion: Follow the given conventions for Git commit messages for future commits (do not modify past commit messages as doing so will change the commit timestamp that we used to detect your commit timings).

Aspect: Binary files in repo

No easy-to-detect 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 [email protected] if you want to follow up on this post.