Skip to content

Sharing iP code quality feedback [for @wpx12011] #1

@nus-se-bot

Description

@nus-se-bot

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

No easy-to-detect issues 👍

Aspect: Brace Style

Example from src/main/java/Duke.java lines 61-62:

            }
            catch (DukeException e) {

Suggestion: As specified by the coding standard, use egyptian style braces.

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.java lines 14-76:

    public static void main(String[] args) {

        String logo = " ____        _        \n"
                + "|  _ \\ _   _| | _____ \n"
                + "| | | | | | | |/ / _ \\\n"
                + "| |_| | |_| |   <  __/\n"
                + "|____/ \\__,_|_|\\_\\___|\n";
        System.out.println("Hello from\n" + logo);
        System.out.println("What can I do for you?\n");

        boolean isOnline = true;
        ArrayList<Task> taskList = new ArrayList<>();

        File savedTasks = new File("saved_tasks_list.txt");
        try {
            if (!savedTasks.createNewFile()) {
                // Load the exist file
                Scanner fileScanner = new Scanner(savedTasks);
                while (fileScanner.hasNextLine()) {
                    String record = fileScanner.nextLine();
                    String[] fields = record.split("\\|");
                    Task currTask;
                    if (fields[0].equals("T")) {
                        currTask = new Todo(fields[2]);
                    } else if (fields[0].equals("D")) {
                        currTask = new Deadline(fields[2], fields[3]);
                    } else if (fields[0].equals("E")) {
                        currTask = new Event(fields[2], fields[3], fields[4]);
                    } else {
                        currTask = new Task("");
                    }

                    if (fields[1].equals("1")) {
                        currTask.setDone();
                    } else {
                        currTask.setNotDone();
                    }
                    taskList.add(currTask);
                }
            }
        } catch (IOException e) {
            e.printStackTrace();
        }

        while (isOnline) {
            try {
                isOnline = takeRequest(taskList);
            }
            catch (DukeException e) {
                System.out.println(e.getMessage());
            }
        }

        try {
            FileWriter writer = new FileWriter("saved_tasks_list.txt");
            for (int i = 0; i < taskList.size(); i++) {
                writer.write(taskList.get(i).toRecord() + "\n");
            }
            writer.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

Example from src/main/java/Duke.java lines 78-244:

    public static boolean takeRequest(ArrayList<Task> taskList) throws DukeException {
        Scanner keyboard = new Scanner(System.in);
        String userInput;

        while (true) {
            userInput = keyboard.next();
            System.out.println("--------------------------------");
            switch (userInput) {
                case "bye": {
                    System.out.println("Bye. Have a nice Day~");
                    return false;
                }
                case "list": {
                    userInput = keyboard.nextLine().trim();
                    if (userInput.isEmpty()) {
                        if (taskList.size() == 0) {
                            System.out.println("The task list is empty.");
                        } else {
                            System.out.println("Here are the current tasks:");
                            for (int i = 0; i < taskList.size(); i++) {
                                System.out.print((i + 1) + ".");
                                System.out.println(taskList.get(i).toString());
                            }
                        }
                    } else {
                        System.out.println("Please key in [list] to check the task list.");
                    }
                    break;
                }
                case "todo": {
                    String name = keyboard.nextLine();
                    if (name.trim().isEmpty()){
                        System.out.println("Adding new todo failed");
                        System.out.println("The task name cannot be empty");
                    } else {
                        System.out.println("This task had been added!");
                        Todo t = new Todo(name);
                        System.out.println("  " + t);
                        taskList.add(t);
                        System.out.println("Now you have " + taskList.size() + " tasks in the list");
                    }
                    break;

                }
                case "event": {
                    userInput = keyboard.nextLine();
                    if (userInput.trim().isEmpty()){
                        System.out.println("The task name cannot be empty");
                        System.out.println("Please enter the task with the format [name /ddMMyyyy HHmm /ddMMyyyy HMmm]");
                    } else {
                        try {
                            int first = userInput.indexOf('/');
                            int last = userInput.lastIndexOf('/');
                            String eventName = userInput.substring(0, first);
                            String from = userInput.substring(first + 1, last-1);
                            String to = userInput.substring(last + 1);
                            try {
                                DateTimeFormatter dateformatter = DateTimeFormatter.ofPattern("ddMMuuuu HHmm");
                                LocalDateTime fromDate = LocalDateTime.parse(from, dateformatter);
                                LocalDateTime toDate = LocalDateTime.parse(to, dateformatter);

                                System.out.println("This event had been added! Hope you will enjoy it :D");

                                Event t = new Event(eventName, fromDate, toDate);
                                System.out.println("  " + t);

                                taskList.add(t);
                                System.out.println("Now you have " + taskList.size() + " tasks in the list");
                            } catch (DateTimeParseException e) {
                                System.out.println("Adding new event failed");
                                System.out.println("Please enter the task with the format [name /ddMMyyyy HHmm /ddMMyyyy HMmm]");
                            }
                        } catch (StringIndexOutOfBoundsException e){
                            System.out.println("Adding new event failed");
                            System.out.println("Please enter the event with the format [name /ddMMyyyy HHmm /ddMMyyyy HMmm]");
                        }
                    }
                    break;

                }
                case "deadline": {
                    userInput = keyboard.nextLine();
                    if (userInput.trim().isEmpty()){
                        System.out.println("The task name cannot be empty");
                        System.out.println("Please enter the task with format [name /ddmmyyyy time]");
                    } else {
                        try {
                            int dateID = userInput.indexOf('/');
                            String eventName = userInput.substring(0, dateID);
                            String details = userInput.substring(dateID + 1);

                            try {
                                DateTimeFormatter dateformatter = DateTimeFormatter.ofPattern("ddMMuuuu HHmm");
                                LocalDateTime by = LocalDateTime.parse(details, dateformatter);
                                System.out.println("This deadline had been added! Try to finish it early 0v0");
                                Deadline t = new Deadline(eventName, by);
                                System.out.println("  " + t);

                                taskList.add(t);
                                System.out.println("Now you have " + taskList.size() + " tasks in the list");
                            } catch (DateTimeParseException e) {
                                System.out.println("Adding new deadline failed");
                                System.out.println("Please enter the task with format [name /ddmmyyyy time]");
                            }

                        } catch (StringIndexOutOfBoundsException e){
                            System.out.println("Adding new deadline failed");
                            System.out.println("Please enter the task with format [name /ddmmyyyy time]");
                        }
                    }
                    break;

                }
                case "mark": {
                    try {
                        int itemID = Integer.parseInt(keyboard.nextLine().trim());
                        if ((itemID) > taskList.size()) {
                            System.out.println("I cannot find task " + (itemID) + " as it exceeds the total tasks number");
                        } else {
                            System.out.println("Nice! Great job for completing this task:");
                            taskList.get(itemID - 1).setDone();
                            System.out.println((taskList.get(itemID - 1).toString()));
                        }
                    } catch (NumberFormatException e) {
                        System.out.println("Please enter the task number");
                    }
                    break;

                }
                case "unmark": {
                    try {
                        int itemID = Integer.parseInt(keyboard.nextLine().trim()) - 1;
                        if ((itemID + 1) > taskList.size()) {
                            System.out.println("I cannot find task " + (itemID + 1) + " as it exceeds the total tasks number");
                        } else {
                            System.out.println("This item is marked as not done yet");
                            taskList.get(itemID).setNotDone();
                            System.out.println((taskList.get(itemID).toString()));
                        }
                    } catch (NumberFormatException e) {
                        System.out.println("Please enter the task number");
                    }
                    break;
                }
                case "delete": {
                    try {
                        int itemID = Integer.parseInt(keyboard.nextLine().trim()) - 1;
                        if ((itemID + 1) > taskList.size()) {
                            System.out.println("I cannot find task " + (itemID + 1) + " as it exceeds the total tasks number");
                        } else {
                            System.out.println("This task is deleted from the list:");
                            System.out.println("  " + (taskList.get(itemID).toString()));
                            taskList.remove(itemID);
                            System.out.println("Now you have " + taskList.size() + " tasks in the list");
                        }
                    } catch (NumberFormatException e) {
                        System.out.println("Please enter the task number");
                    }
                    break;
                }
                default: {
                    throw new DukeException("May I know what type of task this is?");
                }
            }
            System.out.println("--------------------------------");
        }
    }

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 537d5bb:

Completed Level-8

  • Not in imperative mood (?)

possible problems in commit d145450:

Completed Level-7

  • Not in imperative mood (?)

possible problems in commit d3ce3a4:

Completed Level-6

  • Not in imperative mood (?)

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.

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