Skip to content

Sharing iP code quality feedback [for @ZaydM18] - Round 2 #3

Description

@soc-se-bot-blue

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

No easy-to-detect issues 👍

Aspect: Method Length

Example from src/main/java/monty/ui/MainWindow.java lines 89-219:

    private void handleUserInput() {
        String input = userInput.getText().trim();
        if (input.isEmpty()) {
            return;
        }

        String response;
        try {
            StringBuilder capturedOutput = new StringBuilder();

            Ui guiUi = new Ui() {
                @Override
                public void showError(String message) {
                    capturedOutput.append("❌ ").append(message).append("\n");
                }

                @Override
                public void showTaskAdded(Task task, int size) {
                    capturedOutput.append("✔ Task added: ").append(task.toString()).append("\n")
                            .append("Now you have ").append(size).append(" tasks in the list.\n")
                            .append("But what will you do about them?");
                }

                @Override
                public void showTaskDeleted(Task task, int size) {
                    capturedOutput.append("🗑 Task removed: ").append(task.toString()).append("\n")
                            .append("Now you have ").append(size).append(" tasks in the list.\n");
                }

                @Override
                public void showTaskMarked(Task task) {
                    capturedOutput.append("✅ Task marked as done: ").append(task.toString()).append("\n")
                            .append("Impressive, you're more powerful than I expected.");
                }

                @Override
                public void showTaskUnmarked(Task task) {
                    capturedOutput.append("🔄 Task marked as not done: ").append(task.toString()).append("\n")
                            .append("It's not like you to leave business unfinished...").append("\n");
                }

                @Override
                public void showTaskList(java.util.List<Task> tasks) {
                    capturedOutput.append("📋 Here are the tasks in your list:\n");
                    for (int i = 0; i < tasks.size(); i++) {
                        capturedOutput.append((i + 1)).append(". ").append(tasks.get(i)).append("\n");
                    }
                    capturedOutput.append("...\n");
                }

                @Override
                public void showNoTasksFoundForDate() {
                    capturedOutput
                            .append("📅 No deadlines or events found on this date. "
                                    + "Maybe you should make a date with destiny?\n");
                }

                @Override
                public void showTasksForDate(LocalDate date, java.util.List<Task> tasks) {
                    capturedOutput.append("📅 Here are the deadlines and events on ")
                            .append(date.format(DateTimeFormatter.ofPattern("MMM dd yyyy"))).append(":\n");
                    for (Task task : tasks) {
                        capturedOutput.append("  ").append(task).append("\n");
                    }
                }

                @Override
                public void showFoundTasks(Task... matchingTasks) {
                    if (matchingTasks.length == 0) {
                        capturedOutput.append("🔍 No matching tasks found.\n");
                    } else {
                        capturedOutput.append("🔍 Here are the matching tasks:\n");
                        for (int i = 0; i < matchingTasks.length; i++) {
                            capturedOutput.append((i + 1)).append(". ").append(matchingTasks[i]).append("\n");
                        }
                        capturedOutput.append("...\n");
                    }
                }

                @Override
                public void showSortedTasks(ArrayList<Task> tasks, String message) {
                    capturedOutput.append("🔽 ").append(message).append("\n");
                    for (int i = 0; i < tasks.size(); i++) {
                        capturedOutput.append((i + 1)).append(". ").append(tasks.get(i)).append("\n");
                    }
                }

                @Override
                public void showGoodbye() {
                    capturedOutput.append("👋 Bye. Hope to see you again soon! Don't keep me waiting... Cloud. \n");
                }

                @Override
                public void showMessage(String message) {
                    capturedOutput.append(message).append("\n");
                }

                @Override
                public void showHelp() {
                    capturedOutput.append(" For a full list of commands, visit: https://zaydm18.github.io/ip/\n");
                }


            };

            Parser.processCommand(input, tasks, guiUi);
            Storage.saveTasks(tasks);
            response = capturedOutput.toString();

            if (input.equalsIgnoreCase("bye")) {
                appendToDialog("Monty", response);
                userInput.clear();

                exitAfterDelay();
                return;
            }


        } catch (IllegalArgumentException e) {
            response = "❌ Invalid date format! Please use yyyy-MM-dd HHmm.\n";
        } catch (MontyException e) {
            response = "❌ " + e.getMessage();
        } catch (Exception e) {
            response = "❌ An unexpected error occurred.";
            e.printStackTrace();
        }

        appendToDialog("You", input);
        appendToDialog("Monty", response);
        userInput.clear();
    }

Example from src/main/java/monty/parser/Parser.java lines 30-175:

    public static void processCommand(String userInput, ArrayList<Task> tasks, Ui ui) throws MontyException {
        String[] words = userInput.split(" ", 2);
        String command = words[0];
        String argument = words.length > 1 ? words[1].trim() : "";

        switch (command) {
        case "bye": {
            ui.showGoodbye();
            Storage.saveTasks(tasks);
            return;
        }

        case "list": {
            ui.showTaskList(tasks);
            break;
        }

        case "mark": {
            int markIndex = validateTaskIndex(argument, tasks.size());
            tasks.get(markIndex).markAsDone();
            ui.showTaskMarked(tasks.get(markIndex));
            Storage.saveTasks(tasks);
            break;
        }

        case "unmark": {
            int unmarkIndex = validateTaskIndex(argument, tasks.size());
            tasks.get(unmarkIndex).markAsNotDone();
            ui.showTaskUnmarked(tasks.get(unmarkIndex));
            Storage.saveTasks(tasks);
            break;
        }

        case "todo": {
            if (argument.isEmpty()) {
                throw new MontyException(
                        "Huh? You just left that description blank, friend. How can one make a list with this?");
            }

            Task newToDo = new ToDo(argument);
            tasks.add(newToDo);
            ui.showTaskAdded(newToDo, tasks.size());
            Storage.saveTasks(tasks);
            break;
        }

        case "deadline": {
            if (!argument.contains(" /by ")) {
                throw new MontyException(
                        "Deadlines must include a '/by' followed by a date and time (yyyy-MM-dd HHmm).");
            }

            String[] deadlineParts = argument.split(" /by ", 2);
            Task newDeadline = new Deadline(deadlineParts[0], deadlineParts[1]);
            tasks.add(newDeadline);
            ui.showTaskAdded(newDeadline, tasks.size());
            Storage.saveTasks(tasks);
            break;
        }

        case "event": {
            if (!argument.contains(" /from ") || !argument.contains(" /to ")) {
                throw new MontyException(
                        "Events must include '/from' and '/to' with a date and time (yyyy-MM-dd HHmm).");
            }

            String[] eventParts = argument.split(" /from | /to ", 3);
            Task newEvent = new Event(eventParts[0], eventParts[1], eventParts[2]);
            tasks.add(newEvent);
            ui.showTaskAdded(newEvent, tasks.size());
            Storage.saveTasks(tasks);
            break;
        }

        case "date": {
            processDateCommand(argument, tasks, ui);
            break;
        }

        case "delete": {
            if (argument.isEmpty()) {
                throw new MontyException("Please specify a task number to delete.");
            }

            int deleteIndex = validateTaskIndex(argument, tasks.size());
            pendingDeletionTask = tasks.get(deleteIndex);

            ui.showMessage("Are you sure you want to delete this task?\n"
                    + pendingDeletionTask + "\nType 'confirm delete' to proceed.");
            break;
        }

        case "find": {
            processFindCommand(argument, tasks, ui);
            break;
        }

        case "sort": {
            processSortCommand(tasks, ui);
            break;
        }

        case "clear": {
            if (!argument.isEmpty()) {
                throw new MontyException("To delete specific tasks, use the delete command (e.g., delete 1).");
            }

            ui.showMessage("⚠ Are you sure you want to clear all tasks? "
                    + "Don't just erase my precious list willy-nilly! "
                    + "Type 'confirm clear' to proceed.");

            break;
        }

        case "confirm": {
            if (argument.equals("clear")) {
                tasks.clear();
                Storage.saveTasks(tasks);
                ui.showMessage("✅ All tasks have been cleared!");
            } else if (argument.equals("delete")) {
                if (pendingDeletionTask != null) {
                    tasks.remove(pendingDeletionTask);
                    Storage.saveTasks(tasks);
                    ui.showTaskDeleted(pendingDeletionTask, tasks.size());
                    pendingDeletionTask = null;
                } else {
                    throw new MontyException("No pending task deletion found.");
                }
            } else {
                throw new MontyException("Invalid confirmation command.");
            }
            break;
        }

            case "help": {
                ui.showHelp();
                break;
            }

            default: {
            throw new MontyException(
                    "Sorry, did you say that right? If not, please, do correct yourself! The list must expand!");
        }

        }
    }

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/monty/Monty.java lines 58-62:

    /**
     * The entry point for the Monty application.
     *
     * @param args Command-line arguments (not used in this application).
     */

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 Messages

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions