Skip to content

Sharing iP code quality feedback [for @ChickenChiang]Β #1

Description

@soc-se-bot-red

@ChickenChiang 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.java lines 130-130:

        boolean loop = true;

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

Example from src/main/java/Tasks/Deadline.java lines 1-1:

package Tasks;

Example from src/main/java/Tasks/Event.java lines 1-1:

package Tasks;

Example from src/main/java/Tasks/Task.java lines 1-1:

package Tasks;

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/Duke.java lines 119-242:

    public static void main(String[] args) {
        String logo = " _____  _               _____   _\n"
                + "|  ___|| |     _    _  |  ___| | |  _\n"
                + "| |    | |    | |  | | | |     | |/ /\n"
                + "| |___ | |___ | |__| | | |___  |    \\\n"
                + "|_____||_____||______| |_____| |_| \\_\\\n";

        System.out.println(logo);
        System.out.println("    Howdy! I'm Cluck!\n"
                + "    What can I cluck-a-doodle-do for you?");

        boolean loop = true;
        ArrayList<Task> toDoList = readSave();
        Scanner sc = new Scanner(System.in);

        while (loop) {
            String input = sc.nextLine();
            System.out.println(input);
            String[] words = input.split(" ");

            switch (words[0]) {
            case "bye":
                writeSave(toDoList);
                System.out.println("    Buh-cluck, see ya!");
                loop = false;
                break;

            case "list":
                System.out.println("    Here are the tasks in your list:");
                for (int i = 0; i < toDoList.size(); i++) {
                    System.out.println("    " + (i + 1) + ": " + toDoList.get(i).toString());
                }
                break;

            case "mark":
                if (words.length == 1) {
                    System.out.println("    Mucka blucka - Buh cluck! Which task do you wanna mark?");
                } else if (isNumeric(words[1])) {
                    Integer itemNumber = Integer.parseInt(words[1]);
                    if (itemNumber > toDoList.size() || itemNumber <= 0) {
                        System.out.println("    That's not...? In the list...? Buh caw?");
                    } else {
                        toDoList.get(itemNumber - 1).mark();
                        System.out.println("    Marked it! Cluck-a-doodle-done!\n"
                                + toDoList.get(itemNumber - 1).toString());
                    }
                } else {
                    System.out.println("    Ya gotta give me a working number, bucko!");
                }
                break;

            case "unmark":
                if (words.length == 1) {
                    System.out.println("    Which task do you wanna unmark? Muckah buck!");
                } else if (isNumeric(words[1])) {
                    Integer itemNumber = Integer.parseInt(words[1]);
                    if (itemNumber > toDoList.size() || itemNumber <= 0) {
                        System.out.println("    That's not...? In the list...? Buh caw?");
                    } else {
                        toDoList.get(itemNumber - 1).unmark();
                        System.out.println("    Unmarked it! Cluckiddy cluck!\n"
                                + toDoList.get(itemNumber - 1).toString());
                    }
                } else {
                    System.out.println("    Ya gotta give me a working number, bucko!");
                }
                break;

            case MAKE_TODO:
                Task newTodo = new ToDo(input.substring(5));
                toDoList.add(newTodo);
                System.out.println("    added todo:\n    " + newTodo.toString());
                System.out.println("    Now there's " + toDoList.size() + " items in your list!");
                break;

            case MAKE_DEADLINE:
                String body = input.substring(9);
                if (body.contains(DUE_DATE_FLAG)) {
                    String[] fields = body.split(" " + DUE_DATE_FLAG);
                    String description = fields[0];
                    String dueDate = fields[1];
                    Task currDeadline = new Deadline(description, dueDate);
                    toDoList.add(currDeadline);
                    System.out.println("    added deadline: " + currDeadline.toString());
                    System.out.println("    Now there's " + toDoList.size() + " items in your list!");
                    break;
                }
                System.out.println("    You're missing the '/by' flag, bucko!");
                break;

            case MAKE_EVENT:
                String substring = input.substring(6);
                if (substring.contains(EVENT_START_FLAG) && substring.contains(EVENT_END_FLAG)) {
                    String[] fields = substring.split("\\s/\\w{2,4}\\s");
                    Task currEvent = new Event(fields[0], fields[1], fields[2]);
                    toDoList.add(currEvent);
                    System.out.println("    added event: " + currEvent.toString());
                    System.out.println("    Now there's " + toDoList.size() + " items in your list!");
                    break;
                }
                System.out.println("    You're missing the either the '/from' or '/to' flag, or both! Buhcock!");
                break;

            case "delete":
                if (words.length == 1) {
                    System.out.println("    Mucka blucka - Buh cluck! Which task do you wanna delete?");
                } else if (isNumeric(words[1])) {
                    Integer itemNumber = Integer.parseInt(words[1]);
                    if (itemNumber > toDoList.size() || itemNumber <= 0) {
                        System.out.println("    That's not...? In the list...? Buh caw?");
                    } else {
                        System.out.println("   Buh cuck! Removed the following:\n"
                                + toDoList.get(itemNumber - 1).toString());
                    }
                } else {
                    System.out.println("    Ya gotta give me a working number, bucko!");
                }
                break;

            default:
                System.out.println("    You gotta give me a command!");
            }
        }
    }

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

No easy-to-detect issues πŸ‘

Aspect: Recent Git Commit Message (Subject Only)

possible problems in commit d1feb10:

Using Gradle to automate building and testing for Cluck

  • Not in imperative mood (?)

possible problems in commit 58a0861:

Level-8. Added functionality for deadlines to understand date and time and restructure of package

  • Longer than 72 characters

Suggestion: Follow the given conventions for Git commit messages for future commits (no need to modify past commit messages).

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 cs2103@comp.nus.edu.sg if you want to follow up on this post.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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