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 @greyguy21] #4

Open
nus-se-bot opened this issue Sep 11, 2021 · 0 comments
Open

Sharing iP code quality feedback [for @greyguy21] #4

nus-se-bot opened this issue Sep 11, 2021 · 0 comments

Comments

@nus-se-bot
Copy link

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 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/duke/HelpPage.java lines 43-91:

    public void start() {
        label.setText("Here's some help!");
        TableView tableView = new TableView();
        TableColumn<Command, String> command = new TableColumn<>("Command");
        TableColumn<Command, String> function = new TableColumn<>("Function");
        TableColumn<Command, String> format = new TableColumn<>("Format");

        command.setCellValueFactory(
                new PropertyValueFactory<>("cmd")
        );

        function.setCellValueFactory(
                new PropertyValueFactory<>("fn")
        );

        format.setCellValueFactory(
                new PropertyValueFactory<>("format")
        );

        tableView.getColumns().add(command);
        tableView.getColumns().add(function);
        tableView.getColumns().add(format);

        ObservableList data = FXCollections.observableArrayList(
                new Command("bye", "exit app", "bye"),
                new Command("list", "list all functions", "list"),
                new Command("todo", "add tasks without any date/time attached",
                        "todo {task name}"),
                new Command("deadline", "add tasks that need to be done before a specific date & time\n",
                        "deadline {task name} /by {yyyy-MM-dd HH:mm}"),
                new Command("event", "add tasks that need that starts at a specific time\n",
                        "event {task name} /by {yyyy-MM-dd HH:mm}"),
                new Command("done", "marks specified task as done\n",
                        "done {task number}"),
                new Command("delete", "deletes specified task\n",
                        "delete {task number}"),
                new Command("find", "find a task by searching for a keyword\n",
                        "find {keyword}")
        );

        tableView.setItems(data);
        tableView.setColumnResizePolicy(TableView.CONSTRAINED_RESIZE_POLICY);
        VBox vBox = new VBox();
        vBox.getChildren().addAll(label,tableView, exitButton);

        Scene scene = new Scene(vBox);
        stage.setScene(scene);
        stage.show();
    }

Example from src/main/java/duke/parser/Parser.java lines 42-113:

    public String parse(String input) throws DukeException {
               if (input.equals("bye")) {
                   return ui.showBye();
               }

               if (input.equals("list")) {
                   return ui.showTaskList(this.tasks, "list");
               }

               if (input.contains("done")) {
                   int taskNum = getNum(input);
                   Task task = tasks.getTask(taskNum);
                   task.markAsDone();
                   storage.updateData(tasks);
                   return ui.showTaskDone(task);
               }

               if (input.contains("todo")) {
                   String name = getName(input);
                   if (!name.equals("")) {
                       ToDo toDo = new ToDo(name);
                       tasks.addTask(toDo);
                       storage.updateData(tasks);
                       return ui.showTaskAdded(toDo, tasks.getSize());
                   }
               }

               if (input.contains("deadline")) {
                   String description = getName(input);
                   if (checkDescriptionIsValid(description, "/by", "deadline")) {
                       String[] parts = description.split("/by");
                       String name = parts[0];
                       LocalDateTime by = getDateTime(parts[1]);

                       Deadline deadline = new Deadline(name, by);
                       tasks.addTask(deadline);
                       storage.updateData(tasks);
                       return ui.showTaskAdded(deadline, tasks.getSize());
                   }
               }

               if (input.contains("event")) {
                   String description = getName(input);
                   if (checkDescriptionIsValid(description, "/at", "event")) {
                       String[] parts = description.split("/at");
                       String name = parts[0];
                       LocalDateTime at = getDateTime(parts[1]);

                       Event event = new Event(name, at);
                       tasks.addTask(event);
                       storage.updateData(tasks);
                       return ui.showTaskAdded(event, tasks.getSize());
                   }
               }

               if (input.contains("delete")) {
                   int taskNum = getNum(input);
                   Task task = tasks.getTask(taskNum);
                   tasks.deleteTask(task);
                   storage.updateData(tasks);
                   return ui.showTaskDeleted(task, tasks.getSize());
               }

               if (input.contains("find")) {
                   String keyword = getName(input);
                   if (!keyword.equals("")) {
                       TaskList t = tasks.matchTasks(keyword);
                       return ui.showTaskList(t, "find");
                   }
               }
               return invalidEntry();
    }

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: Header Comments

No easy-to-detect issues 👍

Aspect: Recent Git Commit Message (Subject Only)

possible problems in commit d306b9e:

added help page

  • Not in imperative mood (?)

possible problems in commit 9b7296a:

Merge branch 'branch-A-CodeQuality' of https://github.com/greyguy21/ip into branch-A-CodeQuality

  • Longer than 72 characters

Suggestion: Follow the given conventions for Git commit messages

ℹ️ The bot account @cs2103-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