Skip to content

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

Description

@nus-se-bot

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

Example from src/test/java/loafy/parser/ParserTest.java lines 19-19:

        // single digits for date and month results in correct LocalDateTime;

Example from src/test/java/loafy/parser/ParserTest.java lines 24-24:

        // padded date and month results in correct LocalDateTime;

Example from src/test/java/loafy/parser/ParserTest.java lines 29-29:

        // padded time and hour results in correct LocalDateTime;

Suggestion: Remove dead code from the codebase.

Aspect: Method Length

Example from src/main/java/Loafy/parser/Parser.java lines 24-100:

    public static Command parse(String line) throws LoafyException {
        String[] arr = line.split(" ");

        if (arr.length == 0) {
            throw LoafyException.ofEmptyInput();
        } else if (line.equals("bye")) {
            return new ExitCommand();
        } else if (line.equals("list")) {
            return new ListCommand();
        } else if (arr[0].equals("mark")
                || arr[0].equals("unmark")
                || arr[0].equals("delete")) {
            if (arr.length != 2) {
                throw LoafyException.ofInvalidAction();
            } else {
                try {
                    int taskId = Integer.parseInt(arr[1]);
                    if (arr[0].equals("delete")) {
                        return new DeleteCommand(taskId);
                    } else {
                        boolean isDone = arr[0].equals("mark");
                        return new MarkCommand(isDone, taskId);
                    }
                } catch (NumberFormatException e) {
                    throw LoafyException.ofInvalidAction();
                }
            }
        } else if (arr[0].equals("todo")) {
            if (arr.length == 1) {
                throw LoafyException.ofEmptyTodo();
            } else {
                String name = joinRange(arr, 1, arr.length);
                Task task = new Todo(name);
                return new AddCommand(task);
            }
        } else if (arr[0].equals("deadline")) {
            int i = Arrays.asList(arr).indexOf("/by");
            if (i == -1) {
                throw LoafyException.ofNoDeadline();
            } else {
                String name = joinRange(arr, 1, i);
                String date = joinRange(arr, i + 1, arr.length);
                if (name.isEmpty() || date.isEmpty()) {
                    throw LoafyException.ofNoDeadline();
                } else {
                    LocalDateTime dateTime = parseDate(date);
                    Task task = new Deadline(name, dateTime);
                    return new AddCommand(task);
                }
            }
        } else if (arr[0].equals("event")) {
            int fromIndex = Arrays.asList(arr).indexOf("/from");
            int toIndex = Arrays.asList(arr).indexOf("/to");
            if (fromIndex == -1 || toIndex == -1) {
                throw LoafyException.ofNoEventDates();
            } else {
                String name = joinRange(arr, 1, fromIndex);
                String startDateString = joinRange(arr, fromIndex + 1, toIndex);
                String endDateString = joinRange(arr, toIndex + 1, arr.length);
                if (name.isEmpty() || startDateString.isEmpty() || endDateString.isEmpty()) {
                    throw LoafyException.ofNoEventDates();
                } else {
                    LocalDateTime startDate = parseDate(startDateString);
                    LocalDateTime endDate = parseDate(endDateString);
                    Task task = new Event(name, startDate, endDate);
                    return new AddCommand(task);
                }
            }
        } else if (arr[0].equals("find")) {
            if (arr.length == 1) {
                throw LoafyException.ofEmptyFind();
            }
            return new FindCommand(arr[1]);
        } else {
            throw LoafyException.ofInvalidCommand();
        }
    }

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

No easy-to-detect issues 👍

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