Skip to content

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

@nus-se-bot

Description

@nus-se-bot

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

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 36-99:

    String execute(String input) throws DukeException {
        String output;
        try {
            String[] inputArray = input.split(" ", 2);
            String s1 = inputArray[0].toLowerCase();
            if (s1.equals("bye")) {
                output = ui.echo("GoodBye! Thanks for using B.H!");
            } else if (s1.equals("list")) {
                output = ui.echo(this.taskList.getList());
            } else if (s1.equals("mark")) {
                int index = Integer.parseInt(inputArray[1]) - 1;
                if (index < this.taskList.getListSize()) {
                    output = ui.echo("Well done! \n" + this.taskList.mark(index));
                } else {
                    output = ui.echo("Index out of range");
                }
            } else if (s1.equals("unmark")) {
                int index = Integer.parseInt(inputArray[1]) - 1;
                output = ui.echo("Oh no! \n" + this.taskList.unMark(index));
            } else if (s1.equals("todo")) {
                String task = inputArray[1];
                Task newTask = new Todo(task);
                this.taskList.addToList(newTask);
                output = ui.echo("Task added: " + newTask.toString() + "\n"
                        + "Now you have " + this.taskList.getListSize() + " tasks in the list");
            } else if (s1.equals("deadline")) {
                String s = inputArray[1];
                String[] arr = s.split("/by");
                String task = arr[0];
                String time = arr[1];
                Task newTask = new Deadline(task, time);
                this.taskList.addToList(newTask);
                output = ui.echo("Task added:" + newTask.toString() + "\n"
                        + "Now you have " + this.taskList.getListSize() + " tasks in the list");
            } else if (s1.equals("event")) {
                String s = inputArray[1];
                String[] arr = s.split("/at");
                String task = arr[0];
                String time = arr[1];
                Task newTask = new Event(task, time);
                this.taskList.addToList(newTask);
                output = ui.echo("Task added:" + newTask.toString() + "\n"
                        + "Now you have " + this.taskList.getListSize() + " tasks in the list");
            } else if (s1.equals("delete")) {
                int index = Integer.parseInt(inputArray[1]) - 1;
                output = ui.echo("Okay, I have remove this task:\n"
                        + this.taskList.deleteTask(index));
            } else if (s1.equals("check")) {
                LocalDate date = LocalDate.parse(inputArray[1]);
                output = ui.echo(this.taskList.checkDate(date));
            } else if (s1.equals("find")) {
                String[] arr = input.split(" ", 2);
                String word = arr[1];
                output = ui.echo(this.taskList.checkWord(word));
            } else {
                output = ui.echo("Wrong input, please check again");
            }
            this.storage.save(this.taskList.getArrayList());
            return output;
        } catch (ArrayIndexOutOfBoundsException e) {
            System.out.println(e.getMessage());
            throw new DukeException(ui.echo("Duke exception!!!"));
        }
    }

Example from src/main/java/duke/Storage.java lines 28-64:

    ArrayList<Task> load() throws DukeException {
        try {
            FileReader reader = new FileReader(filePath);
            BufferedReader bufferedReader = new BufferedReader(reader);
            String input = bufferedReader.readLine();
            while (input != null) {
                String[] task = input.split(" ### ");
                String type = task[0];
                String status = task[1];
                String thing = task[2];
                if (type.equals("T")) {
                    Task newTask = new Todo(thing);
                    if (status.equals("1")) {
                        newTask.mark();
                    }
                    this.list.add(newTask);
                } else if (type.equals("D")) {
                    Task newTask = new Deadline(thing, task[3]);
                    if (status.equals("1")) {
                        newTask.mark();
                    }
                    this.list.add(newTask);
                } else if (type.equals("E")) {
                    Task newTask = new Event(thing, task[3]);
                    if (status.equals("1")) {
                        newTask.mark();
                    }
                    this.list.add(newTask);
                }
                input = bufferedReader.readLine();
            }
            return this.list;
        } catch (IOException | ArrayIndexOutOfBoundsException e) {
            System.out.println(e.getMessage());
            throw new DukeException("Loading error");
        }
    }

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

Example from src/main/java/Duke.java lines 19-22:

    /**
     * You should have your own function to generate a response to user input.
     * Replace this stub with your completed method.
     */

Example from src/main/java/duke/BH.java lines 28-31:

    /**
     * Return a String of line
     * @return String of line
     */

Example from src/main/java/duke/BH.java lines 36-42:

    /**
     * Read the input and execute the command according to the input
     * Return the output as a String
     * @param input Command from users
     * @return String of the result after execuation
     * @throws DukeException
     */

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