Sharing iP code quality feedback [for @yumengtan]

[nus-se-bot]

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

Example from src/main/java/Lucifer/Lucifer.java lines 1-1:

package Lucifer;

Example from src/main/java/Lucifer/Commands/AddCommands.java lines 1-1:

package Lucifer.Commands;

Example from src/main/java/Lucifer/Commands/DeleteCommands.java lines 1-1:

package Lucifer.Commands;

Suggestion: Follow the package naming convention specified by the coding standard.

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/Lucifer/Lucifer.java lines 96-167:

    public void start(Stage stage) {
        //The container for the content of the chat to scroll.
        scrollPane = new ScrollPane();
        dialogContainer = new VBox();
        scrollPane.setContent(dialogContainer);

        userInput = new TextField();
        sendButton = new Button("Send");

        AnchorPane mainLayout = new AnchorPane();
        mainLayout.getChildren().addAll(scrollPane, userInput, sendButton);

        scene = new Scene(mainLayout);

        stage.setScene(scene);
        stage.show();

        //Formatting the window to look as expected
        stage.setResizable(false);
        stage.setMinHeight(600.0);
        stage.setMinWidth(400.0);
        mainLayout.setPrefSize(400.0, 600.0);

        scrollPane.setPrefSize(385, 535);
        scrollPane.setHbarPolicy(ScrollPane.ScrollBarPolicy.NEVER);
        scrollPane.setVbarPolicy(ScrollPane.ScrollBarPolicy.ALWAYS);

        scrollPane.setVvalue(1.0);
        scrollPane.setFitToWidth(true);
        scrollPane.setStyle("-fx-background: transparent; -fx-background-color: transparent");

        dialogContainer.setPrefHeight(Region.USE_COMPUTED_SIZE);

        userInput.setPrefWidth(325.0);

        sendButton.setPrefWidth(55.0);

        AnchorPane.setTopAnchor(scrollPane, 1.0);
        AnchorPane.setBottomAnchor(sendButton, 1.0);
        AnchorPane.setRightAnchor(sendButton, 1.0);
        AnchorPane.setLeftAnchor(userInput, 1.0);

        //Add functionality to handle user input.
        sendButton.setOnMouseClicked((event) -> {
            dialogContainer.getChildren().add(getDialogLabel(userInput.getText()));
            userInput.clear();
        });

        userInput.setOnAction((event) -> {
            dialogContainer.getChildren().add(getDialogLabel(userInput.getText()));
            userInput.clear();
        });

        sendButton.setOnMouseClicked((event) -> {
            try {
                handleUserInput();
            } catch (IOException | EmptyInputException e) {
                e.printStackTrace();
            }
        });

        userInput.setOnAction((event) -> {
            try {
                handleUserInput();
            } catch (IOException | EmptyInputException e) {
                e.printStackTrace();
            }
        });

        //Scroll down to the end every time dialogContainer's height changes.
        dialogContainer.heightProperty().addListener((observable) -> scrollPane.setVvalue(1.0));
    }

Example from src/main/java/Lucifer/Commands/ParseCommands.java lines 31-63:

    public String parseCommand(String command) {
        DeleteCommands deleter = new DeleteCommands(this.task);
        MarkCommands marker = new MarkCommands(this.task);
        FindCommands finder = new FindCommands(this.task);
        AddCommands adder = new AddCommands(this.task);
        String[] commands = command.split(" ");
        String theCommand = commands[0].toLowerCase();
        String output = null;
        if (command.equals("list")) {
            output = task.giveList();
        } else if (command.equals("!help")) {
            output = Ui.helpCommands();
        } else if (Arrays.stream(WORD_COMMANDS).anyMatch(command::contains)) {
            if (theCommand.equals("mark")) {
                output = marker.mark(command);
            } else if (theCommand.equals("unmark")) {
                output = marker.unMark(command);
            } else if (theCommand.equals("delete")) {
                output = deleter.delete(command);
            } else if (theCommand.equals("find")) {
                output = finder.find(commands[1]);
            } else if (theCommand.equals("todo")) {
                output = adder.addToDo(command);
            } else if (theCommand.equals("deadline")) {
                output = adder.addDeadline(command);
            } else if (theCommand.equals("event")) {
                output = adder.addEvent(command);
            } else if (theCommand.equals("clear")) {
                output = deleter.clear(command);
            }
        }
        return output;
    }

Example from src/main/java/Lucifer/Storage/Storage.java lines 60-114:

    public ArrayList<Task> loadList() throws IOException {
        String directory = filePath + "/Lucifer.Lucifer.txt";
        File file = new File(directory);
        ArrayList<Task> list = new ArrayList<>();
        try {
            Files.createFile(Paths.get(directory));
        } catch (FileAlreadyExistsException e) {
            Scanner sc = new Scanner(file);
            while(sc.hasNext()) {
                String curr = sc.nextLine();
                Character eventType = curr.charAt(0);
                if (eventType.equals('E')) {
                    Character markOrNot = curr.charAt(3);
                    String theTask = curr.substring(6);
                    String[] getTask = theTask.split("\\|");
                    DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd-MM-yyyy HHmm");
                    LocalDateTime old = LocalDateTime.parse(getTask[1].trim(), formatter);
                    Event currTask = new Event(getTask[0].trim(), old);
                    if (markOrNot.equals("1")) {
                        currTask.markDone();
                    } else {
                        currTask.unmarkDone();
                    }
                    list.add(currTask);

                } else if (eventType.equals('D')) {
                    Character markOrNot = curr.charAt(3);
                    String theTask = curr.substring(6);
                    String[] getTask = theTask.split("\\|");
                    DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd-MM-yyyy HHmm");
                    LocalDateTime old = LocalDateTime.parse(getTask[1].trim(), formatter);
                    Deadline currTask = new Deadline(getTask[0].trim(),old);
                    if (markOrNot.equals("1")) {
                        currTask.markDone();
                    } else {
                        currTask.unmarkDone();
                    }
                    list.add(currTask);

                } else if (eventType.equals('T')) {
                    Character markOrNot = curr.charAt(3);
                    int i = curr.lastIndexOf("|") + 2;
                    String theTask = curr.substring(i);
                    Todo currTask = new Todo(theTask);
                    if (markOrNot.equals("1")) {
                        currTask.markDone();
                    } else {
                        currTask.unmarkDone();
                    }
                    list.add(currTask);
                }
            }
        }
        return list;
    }

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/Lucifer/Lucifer.java lines 87-91:

    /**
     * The main method of Lucifer.Lucifer chatbot.
     *
     * @param args The input arguments.
     */

Example from src/main/java/Lucifer/Lucifer.java lines 168-174:

    /**
       * Iteration 1:
       * Creates a label with the specified text and adds it to the dialog container.
       *
       * @param text String containing text to add
       * @return a label with the specified text that has word wrap enabled.
     */

Example from src/main/java/Lucifer/Lucifer.java lines 181-185:

    /**
     * Iteration 2:
     * Creates two dialog boxes, one echoing user input and the other containing Duke's reply and then appends them to
     * the dialog container. Clears the user input after processing.
     */

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.