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 @Denniszedead] - Round 2 #11

Open
nus-se-bot opened this issue Mar 15, 2022 · 0 comments
Open

Sharing iP code quality feedback [for @Denniszedead] - Round 2 #11

nus-se-bot opened this issue Mar 15, 2022 · 0 comments

Comments

@nus-se-bot
Copy link

@Denniszedead We did an automated analysis of your code to detect potential areas to improve the code quality. We are sharing the results below, so that you can avoid similar problems in your tP code (which will be graded more strictly for code quality).

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/test/java/duke/command/TestCommand.java lines 103-153:

    void createMarkCommand_markCommandWithNumber_taskMarked() {
        String initial = "TODO\n"
                + "false\n"
                + "Task 1\n"
                + "*** Next Task ***\n";
        String expectedOne = "Marked the following task to the list:\n"
                + "[T][X] Task 1"
                + "\nGood job by the way";
        String expectedTwo = "TODO\n"
                + "true\n"
                + "Task 1\n"
                + "*** Next Task ***\n";
        String fileName = "data/test/dukeMarkCmdTest.txt";

        try {
            // Initialize file data.
            Storage storage = new Storage(fileName);
            TaskList tasks = new TaskList();
            File dataFile = new File(fileName);
            Scanner sc = new Scanner(dataFile);
            FileWriter fw = new FileWriter(dataFile);

            // Add the initial data into the data file.
            tasks.addTask(Task.createTask("TODO", false, "Task 1", null));
            storage.updateData(tasks);
            String result = "";
            while (sc.hasNext()) {
                result += sc.nextLine() + "\n";
            }
            Assertions.assertEquals(initial, result);

            // Checks whether the command is created.
            Command cmd = Command.getCommand("MARK", "1");
            Assertions.assertEquals(expectedOne, cmd.run(tasks, storage));

            // Checks whether the dataFile is updated.
            result = "";
            sc = new Scanner(dataFile);
            while (sc.hasNext()) {
                result += sc.nextLine() + "\n";
            }
            Assertions.assertEquals(expectedTwo, result);

            // Clear the file.
            fw.write("");
            fw.close();
        } catch (Exception e) {
            System.out.println(e.getMessage());
            Assertions.fail();
        }
    }

Example from src/test/java/duke/command/TestCommand.java lines 174-224:

    void createUnmarkCommandResult_unmarkCommandWithNumber_taskUnmarked() {
        String initial = "TODO\n"
                + "true\n"
                + "Task 1\n"
                + "*** Next Task ***\n";
        String expectedOne = "Unmarked the following task to the list:\n"
                + "[T][ ] Task 1"
                + "\nHope you get it done soon!";
        String expectedTwo = "TODO\n"
                + "false\n"
                + "Task 1\n"
                + "*** Next Task ***\n";
        String fileName = "data/test/dukeUnmarkCmdTest.txt";

        try {
            // Initialize the files.
            Storage storage = new Storage(fileName);
            TaskList tasks = new TaskList();
            File dataFile = new File(fileName);
            Scanner sc = new Scanner(dataFile);
            FileWriter fw = new FileWriter(dataFile);

            // Initialize the data in the data file.
            tasks.addTask(Task.createTask("TODO", true, "Task 1", null));
            storage.updateData(tasks);
            String result = "";
            while (sc.hasNext()) {
                result += sc.nextLine() + "\n";
            }
            Assertions.assertEquals(initial, result);

            // Checks whether the command is created.
            Command cmd = Command.getCommand("UNMARK", "1");
            Assertions.assertEquals(expectedOne, cmd.run(tasks, storage));

            // Checks whether the dataFile is updated.
            result = "";
            sc = new Scanner(dataFile);
            while (sc.hasNext()) {
                result += sc.nextLine() + "\n";
            }
            Assertions.assertEquals(expectedTwo, result);

            // Clears the dataFile.
            fw.write("");
            fw.close();
        } catch (Exception e) {
            System.out.println(e.getMessage());
            Assertions.fail();
        }
    }

Example from src/test/java/duke/command/TestCommand.java lines 346-391:

    void createDeadlineCommand_correctDeadline_createDeadlineInFile() {
        String initial = "";
        String expectedOne =
                "Added the following deadline into the list:\n[D][ ] Deadline 1 (by: 01 Jan 2022 11:59 PM)";
        String expectedTwo = "DEADLINE\n"
                + "false\n"
                + "Deadline 1\n"
                + "1/1/2022 2359\n"
                + "*** Next Task ***\n";

        try {
            Storage storage = new Storage(fileName);
            TaskList tasks = new TaskList();
            File dataFile = new File(fileName);
            Scanner sc = new Scanner(fileName);
            FileWriter fw = new FileWriter(dataFile);

            // Checks whether the file is empty.
            String result = "";
            sc = new Scanner(dataFile);
            while (sc.hasNext()) {
                result += sc.nextLine() + "\n";
            }
            Assertions.assertEquals(initial, result);

            // Check whether the command is created
            Command cmd = Command.getCommand("DEADLINE", "Deadline 1 /by 01/01/2022 2359");
            result = cmd.run(tasks, storage);
            Assertions.assertEquals(expectedOne, result);

            // Checks whether data is saved in the dataFile.
            result = "";
            sc = new Scanner(dataFile);
            while (sc.hasNext()) {
                result += sc.nextLine() + "\n";
            }
            Assertions.assertEquals(expectedTwo, result);

            // Clears the dataFile.
            fw.write("");
            fw.close();
        } catch (Exception e) {
            e.printStackTrace();
            Assertions.fail();
        }
    }

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

Suggestion: Consider breaking large classes into smaller ones, if appropriate. A long class is a sign that perhaps it is dong 'too much'.

Aspect: Header Comments

Example from src/main/java/duke/praser/Parser.java lines 10-16:

    /**
     * Prase the user input and returns the corresponding command.
     *
     * @param userInput The input entered by the user.
     * @return The Command as typed by the user.
     * @throws InvalidCommandException If the user inputs an invalid command.
     */

Example from src/main/java/duke/task/Task.java lines 56-60:

    /**
     * Get the task name of the task.
     *
     * @return The name of the task.
     */

Example from src/main/java/duke/task/Task.java lines 132-136:

    /**
     * {@inheritDoc}
     *
     * @return {@inheritDoc}
     */

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 👍

Aspect: Binary files in repo

No easy-to-detect issues 👍

❗ You are not required to (but you are welcome to) fix the above problems in your iP, unless you have been separately asked to resubmit the iP due to code quality 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 [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