Skip to content

Sharing iP code quality feedback [for @e1121208] #3

@soc-se-bot-blue

Description

@soc-se-bot-blue

@e1121208 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/axel/Parser.java lines 15-57:

    public static Command parse(String command) throws AxelException {
        assert command != null : "Command input should not be null";

        if (command.startsWith("todo")) {
            String taskName = command.substring(5).trim();
            assert !taskName.isEmpty() : "I need a task name...";
            return new AddCommand(new ToDoTask(taskName));

        } else if (command.startsWith("deadline")) {
            String[] parts = command.split(" /by ");
            assert parts.length >= 2 : "Deadline command should contain a '/by'";
            return new AddCommand(new DeadlineTask(parts[0].substring(9).trim(), parts[1].trim()));

        } else if (command.startsWith("event")) {
            String[] parts = command.split(" /from | /to ");
            assert parts.length >= 3 : "Event command should contain a '/from' and '/to'";
            return new AddCommand(new EventTask(parts[0].substring(6).trim(), parts[1].trim(), parts[2].trim()));

        } else if (command.startsWith("mark")) {
            return new MarkCommand(parseTaskIndex(command));

        } else if (command.startsWith("unmark")) {
            return new UnmarkCommand(parseTaskIndex(command));

        } else if (command.startsWith("delete")) {
            return new DeleteCommand(parseTaskIndex(command));

        } else if (command.equalsIgnoreCase("list")) {
            return new ListCommand();

        } else if (command.equalsIgnoreCase("bye")) {
            return new ExitCommand();

        } else if (command.startsWith("find")) {
            return new FindCommand(command.substring(5).trim());

        } else if (command.equalsIgnoreCase("help")) {
            return new HelpCommand();

        } else {
            throw new UnrecognisedCommandException();
        }
    }

Example from src/main/java/axel/Storage.java lines 31-86:

    public List<Task> load() throws IOException, CorruptedFileException {
        List<Task> taskList = new ArrayList<>();
        File file = new File(filePath);

        if (!file.exists()) {
            file.getParentFile().mkdirs();
            file.createNewFile();
        }

        try (BufferedReader reader = new BufferedReader(new FileReader(filePath))) {
            String line;
            while ((line = reader.readLine()) != null) {
                String[] parts = line.split(" \\| ");
                if (parts.length < 3) {
                    throw new CorruptedFileException("Corrupted data in file: " + line);
                }

                Task task;
                String type = parts[0];
                boolean isDone = parts[1].equals("1");
                String description = parts[2];

                switch (type) {
                case "T":
                    task = new ToDoTask(description);
                    break;

                case "D":
                    if (parts.length < 4) {
                        throw new CorruptedFileException("Incomplete DeadlineTask data in file: " + line);
                    }
                    String by = parts[3];
                    task = new DeadlineTask(description, by);
                    break;

                case "E":
                    if (parts.length < 5) {
                        throw new CorruptedFileException("Incomplete EventTask data in file: " + line);
                    }
                    String from = parts[3];
                    String to = parts[4];
                    task = new EventTask(description, from, to);
                    break;

                default:
                    throw new CorruptedFileException("Invalid task type in file: " + type);
                }

                if (isDone) {
                    task.markAsDone();
                }
                taskList.add(task);
            }
        }
        return taskList;
    }

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

Example from src/main/java/axel/Axel.java lines 78-83:

    /**
     * The entry point of the application.
     * Initializes the application and starts the main loop.
     *
     * @param args Command-line arguments passed to the application (not used).
     */

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

possible problems in commit e38c167:


Add a 'help' command for in-app user guidance

> Implemented a HelpCommand class that provides a list of available commands and how to use them.
> Updated the Ui class to include a printHelpMessage() method to display the help information.
> Modified the Parser class to handle the "help" command, allowing users to easily access a list of supported commands.

This change improves the usability of the app by providing clear in-app guidance to users.


  • body not wrapped at 72 characters: e.g., > Implemented a HelpCommand class that provides a list of available commands and how to use them.

possible problems in commit 91f8948:


Merge pull request #1 from e1121208/branch-A-Assertions

Add assert statements to validate important assumptions across the codebase

  • body not wrapped at 72 characters: e.g., Add assert statements to validate important assumptions across the codebase

possible problems in commit ffa86ad:


Add assert statements to validate important assumptions across the codebase

Implemented the assert feature in various parts of the code to document and validate key assumptions, improving robustness and making the code easier to maintain. These assertions are used to catch potential logical errors during development and testing.

Parser Class:
* Added assertions to check if the command input is non-null.
* Verified that ToDo descriptions and Deadline commands contain necessary information.

AddCommand Class:
* Ensured the task being added is non-null before proceeding with the execution.

TaskList Class:
* Added bounds-check assertions to verify that the task index is valid when retrieving tasks.

FindCommand Class:
* Added an assertion to verify that the keyword provided for searching tasks is non-null and non-empty.
* Ensured that the list of matching tasks is not null before attempting to print it.

These assertions will help prevent potential issues from propagating through the codebase and are intended to be enabled during testing or debugging by using the -ea flag when running the application.

The use of assertions ensures that key assumptions hold true, leading to better error detection during development. It helps catch invalid states early and improves code quality by making assumptions explicit.


  • Longer than 72 characters
  • body not wrapped at 72 characters: e.g., Implemented the assert feature in various parts of the code to document and validate key assumptions, improving robustness and making the code easier to maintain. These assertions are used to catch potential logical errors during development and testing.

Suggestion: Follow the given conventions for Git commit messages for future commits (do not modify past commit messages as doing so will change the commit timestamp that we used to detect your commit timings).

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