Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sharing iP code quality feedback [for @tanweien] #3

Open
nus-se-bot opened this issue Feb 11, 2022 · 0 comments
Open

Sharing iP code quality feedback [for @tanweien] #3

nus-se-bot opened this issue Feb 11, 2022 · 0 comments

Comments

@nus-se-bot
Copy link

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

Example from src/main/java/duke/Task.java lines 9-9:

    private boolean completed;

Suggestion: Follow the given naming convention for boolean variables/methods (e.g., use a boolean-sounding prefix).You may ignore the above if you think the name already follows the convention (the script can report false positives in some cases)

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/duke/Parser.java lines 12-106:

    public Command parse(String command) throws DukeException {
        String fullCommand = command.toLowerCase();
        String commandWord = ui.getCommandWord(fullCommand);
        if (fullCommand.equals("bye")) {
            ExitCommand c = new ExitCommand(fullCommand);
            return c;
        } else if (fullCommand.equals("save")) {
            SaveCommand c = new SaveCommand(fullCommand);
            return c;
        } else if (fullCommand.equals("list")) {
            ListCommand c = new ListCommand(fullCommand);
            return c;
        } else if (commandWord.equals("todo")) {
            if (ui.isValidTask(fullCommand)) {
                String taskName = ui.getTaskName(fullCommand);
                ToDo todo = new ToDo(taskName);
                AddCommand addCommand = new AddCommand("todo", todo);
                return addCommand;
            } else {
                throw new EmptyDescriptionException("Todo description cannot be empty!");
            }
        } else if (commandWord.equals("deadline")) {
            if (!ui.isValidTask(fullCommand)) {
                throw new EmptyDescriptionException("Deadline description cannot be empty!");
            } else if (!ui.isValidDeadline(fullCommand)) {
                throw new InvalidCommandFormatException("Deadlines must include \"/by\"");
            } else {
                String taskName = ui.getTaskName(fullCommand);
                LocalDate localDate = ui.getTaskDate(fullCommand);
                Deadline deadline = new Deadline(taskName, localDate);
                AddCommand addCommand = new AddCommand("deadline", deadline);
                return addCommand;
            }


        } else if (commandWord.equals("event")) {
            if (!ui.isValidTask(fullCommand)) {
                throw new EmptyDescriptionException("Event description cannot be empty!");
            } else if (!ui.isValidEvent(fullCommand)) {
                throw new InvalidCommandFormatException("Events must include \"/at\"");
            } else {
                String taskName = ui.getTaskName(fullCommand);
                LocalDate localDate = ui.getTaskDate(fullCommand);
                Event event = new Event(taskName, localDate);
                AddCommand addCommand = new AddCommand("event", event);
                return addCommand;
            }

        } else if (commandWord.equals("mark")) {
            if (ui.isValidMarkFormat(fullCommand)) {
                MarkCommand markCommand = new MarkCommand(fullCommand,
                        ui.markIndex(fullCommand));
                return markCommand;
            } else {
                throw new InvalidCommandFormatException("Please include item index!");
            }
        } else if (commandWord.equals("unmark")) {
            if (ui.isValidUnmarkFormat(fullCommand)) {
                UnmarkCommand unmarkCommand = new UnmarkCommand(fullCommand,
                        ui.markIndex(fullCommand));
                return unmarkCommand;
            } else {
                throw new InvalidCommandFormatException("Please include item index!");
            }
        } else if (commandWord.equals("find")) {
            if (ui.isValidTask(fullCommand)) {
                FindCommand findCommand = new FindCommand(fullCommand);
                return findCommand;
            } else {
                throw new InvalidCommandFormatException("Invalid find!");
            }
        } else if (commandWord.equals("delete")) {
            if (ui.isValidDeleteFormat(fullCommand)); {
                DeleteCommand deleteCommand = new DeleteCommand(fullCommand,
                        ui.deleteIndex(fullCommand));
                return deleteCommand;
            }
        } else if (commandWord.equals("sort")) {
            SortCommand sortCommand = new SortCommand(fullCommand);
            return sortCommand;
        } else {
                String generalFormatExceptionString = "Please give a proper command!\n" +
                        "List of commands: \n" +
                        "1. todo\n" +
                        "2. deadline\n" +
                        "3. event\n" +
                        "4. list\n" +
                        "5. mark\n" +
                        "6. unmark\n" +
                        "7. find\n" +
                        "8. delete \n" +
                        "9. bye";
                throw new InvalidCommandFormatException(generalFormatExceptionString);
        }
    }

Example from src/main/java/duke/Storage.java lines 53-96:

    public TaskList readData() {
        TaskList taskList = new TaskList();
        try {
            Scanner sc = new Scanner(new File(filePath));
            while (sc.hasNext()) {
                String task = sc.nextLine();
                String[] values = task.split(" \\| ");
                String taskType = values[TASK_TYPE];
                String taskCompletion = values[TASK_COMPLETED];
                String taskItem = values[TASK_ITEM];
                switch (taskType) {
                    case "T":
                        ToDo todo = new ToDo(taskItem);
                        if (taskCompletion == "1") {
                            todo.setCompleted();
                        }
                        taskList.add(todo);
                        break;
                    case "E":
                        String eventDate = values[TASK_DATE];
                        LocalDate date = LocalDate.parse(eventDate);
                        Event event = new Event(taskItem, date);
                        if (taskCompletion == "1") {
                            event.setCompleted();
                        }
                        taskList.add(event);
                        break;
                    case "D":
                        String deadlineDate = values[3];
                        LocalDate localDate = LocalDate.parse(deadlineDate);
                        Deadline deadline = new Deadline(taskItem, localDate);
                        if (taskCompletion == "1") {
                            deadline.setCompleted();

                        }
                        taskList.add(deadline);
                        break;
                }
            }
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        }
        return taskList;
    }

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

No easy-to-detect issues 👍

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant