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

[nus-se-script]

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

Example from src/main/java/GUI/DialogBox.java lines 1-1:

package GUI;

Example from src/main/java/GUI/MainWindow.java lines 1-1:

package GUI;

Suggestion: Follow the package naming convention specified by the coding standard.

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/command/AddCommand.java lines 54-147:

    public String execute(TaskList taskList, NotesList notesList, Ui ui, Storage storage) throws DukeException {
        String response = "";
        assert !taskList.isEmpty();
        assert !notesList.isEmpty();
        if (commandLine.startsWith("todo")) {
            try {
                if (commandLine.length() <= 5) {
                    throw new DukeException("The description of a todo cannot be empty.");
                } else {
                    String actTask = commandLine.substring(5);
                    Task currTask = new Todo(actTask);
                    taskList.add(currTask);
                    storage.updateData(taskList, notesList);
                    response = "Got it. I've added this todo task:\n"
                            + " " + currTask + "\n" + "Now you have "
                            + taskList.size() + " tasks in the list.\n";
                    System.out.println(response);
                    return response;
                }
            } catch (DukeException e) {
                System.out.println(e.toString());
                return e.toString();
            }
        } else if (commandLine.startsWith("deadline")) {
            try {
                if (commandLine.length() <= 9) {
                    throw new DukeException("The description of a deadline cannot be empty.");
                } else {
                    if (!commandLine.contains("/")) {
                        throw new DukeException("There is no date! >:(\n");
                    } else {
                        int slashIndex = commandLine.indexOf("/");
                        String actTask = commandLine.substring(9, slashIndex - 1);
                        String taskDate = commandLine.substring(slashIndex + 4);
                        Task currTask = new Deadline(actTask, taskDate);
                        taskList.add(currTask);
                        storage.updateData(taskList, notesList);
                        response = "Got it. I've added this deadline task:\n"
                                + " " + currTask + "\n" + "Now you have "
                                + taskList.size() + " tasks in the list.\n";
                        System.out.println(response);
                        return response;
                    }
                }
            } catch (DukeException e) {
                System.out.println(e.toString());
                return e.toString();
            }
        } else if (commandLine.startsWith("event")) {
            try {
                if (commandLine.length() <= 6) {
                    throw new DukeException("The description of a event cannot be empty.");
                } else {
                    if (!commandLine.contains("/")) {
                        throw new DukeException("There is no date! >:(");
                    } else {
                        int slashIndex = commandLine.indexOf("/");
                        String actTask = commandLine.substring(6, slashIndex - 1);
                        String taskDate = commandLine.substring(slashIndex + 4);
                        Task currTask = new Event(actTask, taskDate);
                        taskList.add(currTask);
                        storage.updateData(taskList, notesList);
                        response = "Got it. I've added this event task:\n"
                                + " " + currTask + "\n" + "Now you have "
                                + taskList.size() + " tasks in the list.\n";
                        System.out.println(response);
                        return response;
                    }
                }
            } catch (DukeException e) {
                System.out.println(e.toString());
                return e.toString();
            }
        } else {
            try {
                if (commandLine.length() <= 6) {
                    throw new DukeException("The description of a notes cannot be empty.");
                } else {
                    String actTask = commandLine.substring(6);
                    Task currNote = new Notes(actTask);
                    notesList.add(currNote);
                    storage.updateData(taskList, notesList);
                    response = "Got it. I've added this note:\n"
                            + " " + currNote + "\n" + "Now you have "
                            + notesList.size() + " notes in the list.\n";
                    System.out.println(response);
                    return response;
                }
            } catch (DukeException e) {
                System.out.println(e.toString());
                return e.toString();
            }
        }
    }

Example from src/main/java/command/EditCommand.java lines 49-98:

    public String execute(TaskList taskList, NotesList notesList, Ui ui, Storage storage) {
        String response = "";
        assert !taskList.isEmpty();
        assert !notesList.isEmpty();
        if (commandLine.startsWith("mark")) {
            int index = Integer.parseInt(commandLine.substring(5)) - 1;
            try {
                if (index > taskList.size() - 1 || index < 0) {
                    throw new DukeException("You have no such tasks.");
                } else {
                    Task task = taskList.get(index);
                    if (task instanceof Notes) {
                        response = "Sorry, you can't mark a note.";
                    } else {
                        task.isMark(true);
                        response = "Nice! I've marked this task as done:\n"
                                + " " + task + "\n";
                    }
                    storage.updateData(taskList, notesList);
                    System.out.println(response);
                    return response;
                }
            } catch (DukeException e) {
                System.out.println(e.toString());
                return e.toString();
            }
        } else {
            int index = Integer.parseInt(commandLine.substring(7)) - 1;
            try {
                if (index > taskList.size() - 1 || index < 0) {
                    throw new DukeException("You have no such tasks.");
                } else {
                    Task task = taskList.get(index);
                    if (task instanceof Notes) {
                        response = "Sorry, you can't unmark a note.";
                    } else {
                        task.isMark(false);
                        storage.updateData(taskList, notesList);
                        response = "OK, I've marked this task as not done yet:\n"
                                + " " + task + "\n";
                    }
                    System.out.println(response);
                    return response;
                }
            } catch (DukeException e) {
                System.out.println(e.toString());
                return e.toString();
            }
        }
    }

Example from src/main/java/command/FindCommand.java lines 41-90:

    public String execute(TaskList taskList, NotesList notesList, Ui ui, Storage storage) throws DukeException {
        String response = "";
        try {
            assert !taskList.isEmpty();
            assert !notesList.isEmpty();
            if (taskList.size() == 0) {
                throw new DukeException("There are no tasks in your list. :)");
            } else {
                String taskToFind = this.fullCommand.substring(5);
                ArrayList<Task> tasksFound = new ArrayList<>(taskList.size());
                for (int i = 0; i < taskList.size(); i++) {
                    Task currTask = taskList.get(i);
                    String currTaskString = currTask.toString();
                    if (currTaskString.contains(taskToFind)) {
                        tasksFound.add(currTask);
                    }
                }
                if (!notesList.isEmpty()) {
                    for (int j = 0; j < taskList.size(); j++) {
                        Task currNote = notesList.get(j);
                        String currNoteString = currNote.toString();
                        if (currNoteString.contains(taskToFind)) {
                            tasksFound.add(currNote);
                        }
                    }
                }
                if (tasksFound.isEmpty()) {
                    throw new DukeException("There is nothing that matches the task.");
                } else {
                    System.out.println("Here are the matching tasks in your list:");
                    for (int l = 0; l < tasksFound.size(); l++) {
                        Task currTask = tasksFound.get(l);
                        String currTaskString = currTask.toString();
                        System.out.println(currTaskString);
                    }
                    System.out.println();
                    response = "Here are the matching tasks in your list:\n";
                    for (int k = 0; k < tasksFound.size(); k++) {
                        Task currTaskFound = tasksFound.get(k);
                        String currTaskFoundString = currTaskFound.toString() + "\n";
                        response += currTaskFoundString;
                    }
                    return response;
                }
            }
        } catch (DukeException e) {
            System.out.println(e.toString());
            return e.toString();
        }
    }

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/dateformat/DateFormat.java lines 52-55:

    /**
     * A formatter to format the date and time
     * into a readable String for the DateTimeFormatterBuilder.
     */

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.