Sharing iP code quality feedback [for @ZhuLeYao] - Round 2

[nus-se-script]

@ZhuLeYao 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/main/java/duke/main/Parser.java lines 57-156:

    public String parse(String command, Ui ui, TaskList allTasks, Storage storage, Storage storageArchive) {

        DateTimeFormatter dateTimeFormatter =
                DateTimeFormatter.ofPattern("yyyy-MM-dd HHmm");

        int taskIndex;

        try {
            if (command.equals("list")) {
                String output;
                output = ui.printCommandList(allTasks.getAllTasks());
                return output;
            } else if (command.startsWith("mark")) {
                taskIndex = checkExceptionAndGetTask(command, allTasks);
                Task oldTask = allTasks.getTask(taskIndex);
                Task task = allTasks.markTaskAsDone(oldTask, taskIndex);
                storage.saveListToFile(command, task, allTasks);
                return task.markAsDone();
            } else if (command.startsWith("unmark")) {
                taskIndex = checkExceptionAndGetTask(command, allTasks);
                Task oldTask = allTasks.getTask(taskIndex);
                Task task = allTasks.unmarkTaskAsUndone(oldTask, taskIndex);
                storage.saveListToFile(command, task, allTasks);
                return task.unmarkAsUndone();
            } else if (command.startsWith("todo")) {
                DukeException.emptyCommandException(command);
                String[] str = command.split("todo");
                String taskName = str[1];
                Todo todo = new Todo(allTasks.getNumberOfTask(), false,
                        taskName, allTasks.getNumberOfTask() + 1);
                allTasks.addTask(todo);
                storage.saveListToFile(command, todo, allTasks);
                storageArchive.saveListToFile(command, todo, allTasks);
                return todo.printToDoTask();
            } else if (command.startsWith("deadline")) {
                DukeException.emptyCommandException(command);
                DukeException.missingTimingException(command);
                String[] str = command.split("/by ");
                String taskName = str[0].split("deadline")[1];
                LocalDateTime taskDeadline = LocalDateTime.parse(str[1], dateTimeFormatter);
                Deadline deadline = new Deadline(allTasks.getNumberOfTask(), false,
                        taskName, taskDeadline, allTasks.getNumberOfTask() + 1);
                allTasks.addTask(deadline);
                storage.saveListToFile(command, deadline, allTasks);
                storageArchive.saveListToFile(command, deadline, allTasks);
                return deadline.printDeadlineTask();
            } else if (command.startsWith("event")) {
                DukeException.emptyCommandException(command);
                DukeException.missingTimingException(command);
                String[] str = command.split("/from ");
                String taskName = str[0].split("event")[1];
                String[] eventStartEndTime = str[1].split(" /to ");
                LocalDateTime eventStartTime = LocalDateTime.parse(eventStartEndTime[0], dateTimeFormatter);
                LocalDateTime eventEndTime = LocalDateTime.parse(eventStartEndTime[1], dateTimeFormatter);
                Event event = new Event(allTasks.getNumberOfTask(), false,
                        taskName, eventStartTime, eventEndTime, allTasks.getNumberOfTask() + 1);
                allTasks.addTask(event);
                storage.saveListToFile(command, event, allTasks);
                storageArchive.saveListToFile(command, event, allTasks);
                return event.printEventTask();
            } else if (command.startsWith("delete")) {
                taskIndex = checkExceptionAndGetTask(command, allTasks);
                Task task = allTasks.getTask(taskIndex);
                allTasks.deleteTask(taskIndex);
                storage.saveListToFile(command, task, allTasks);
                return task.printDelete(allTasks.getAllTasks());
            } else if (command.startsWith("show deadlines or events on")) {
                DukeException.emptyCommandException(command);
                String[] str = command.split("show deadlines or events on ");
                String dateTime = str[1];
                DateTimeFormatter dateTimeFormatter2 =
                        DateTimeFormatter.ofPattern("yyyy-MM-dd");
                LocalDate dateTime1 = LocalDate.parse(dateTime, dateTimeFormatter2);
                return ui.printDeadlineOrEventsOnDay(dateTime1, allTasks);
            } else if (command.startsWith("find")) {
                String[] str = command.split("find");
                String keyword = str[1];
                return ui.printFindResults(keyword, allTasks);
            } else if (command.equals("archive")) {
                Storage storage1 = new Storage("data/archiveAll.txt");
                storage1.loadTxtFile();
                storage1.saveWholeListToFile(allTasks);
                storage.clear();
                allTasks.deleteAllTasks();
                return ui.printArchiveMessage();
            } else if (command.equals("bye")){
                return ui.printByeMessage();
            } else {
                DukeException.invalidCommandException(command);
            }
        } catch (DukeException d) {
            return d.getMessage();
        } catch (NumberFormatException nfe) {
            return "\t ☹ OOPS!!! The task index to delete or un/mark a task cannot be a non-integer.";
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
        assert false: "Uncaught error";
        return "Uncaught error";
    }

Example from src/main/java/duke/main/Storage.java lines 37-88:

    public List<Task> loadTxtFile() throws IOException {

        List<Task> allTasks = new ArrayList<>();

        File file3 = new File(this.filePathParent);
        assert !this.filePath.equals("");
        assert !this.filePathParent.equals(null);
        if (!file3.exists()) {
            file3.mkdir();
        }
        File file = new File(this.filePath);
        if (file.exists()) {
            List<String> taskList = new ArrayList<>();
            FileReader file2 = new FileReader(file);
            BufferedReader bufferedReader = new BufferedReader(file2);
            String taskInformation = bufferedReader.readLine();
            while (taskInformation != null) {
                taskList.add(taskInformation);
                taskInformation = bufferedReader.readLine();
            }
            for (int i = 0; i < taskList.size(); i++) {
                String task = taskList.get(i);
                String[] task1 = task.split(" / ");
                boolean isMarked = false;
                if (task1[1].equals("[ ]")) {
                    isMarked = false;
                } else if (task1[1].equals("[X]")){
                    isMarked = true;
                }
                DateTimeFormatter dateTimeFormatter1 =
                        DateTimeFormatter.ofPattern("MMM dd yyyy HHmm a");
                if (task.startsWith("T")) {
                    Todo todo = new Todo(i + 1, isMarked, task1[2],
                            taskList.size());
                    allTasks.add(todo);
                } else if (task.startsWith("D")) {
                    Deadline deadline = new Deadline(i + 1, isMarked, task1[2],
                            LocalDateTime.parse(task1[3], dateTimeFormatter1), taskList.size());
                    allTasks.add(deadline);
                } else if (task.startsWith("E")) {
                    String[] taskTiming = task1[3].split("-");
                    Event event = new Event(i + 1, isMarked, task1[2],
                            LocalDateTime.parse(taskTiming[0], dateTimeFormatter1),
                            LocalDateTime.parse(taskTiming[1], dateTimeFormatter1), taskList.size());
                    allTasks.add(event);
                } else {
                    assert false: "Erroneous task";
                }
            }
        }
        return allTasks;
    }

Example from src/main/java/duke/main/Storage.java lines 144-192:

    public void saveListToFile(String command, Task task, TaskList taskList) throws IOException {
        File file3 = new File(this.filePathParent);
        if (!file3.exists()) {
            file3.mkdir();
        }
        File file = new File(this.filePath);
        FileWriter file1 = new FileWriter(file, true);
        BufferedWriter buffer = new BufferedWriter(file1);

        DateTimeFormatter dateTimeFormatter1 =
                DateTimeFormatter.ofPattern("MMM dd yyyy HHmm a");

        if (command.startsWith("todo")) {
            String content = "T / " + task.getTaskStatus() + " / " + task.getTask() + "\n";
            buffer.write(content);
        } else if (command.startsWith("deadline")) {
            String content = "D / " + task.getTaskStatus() + " / "
                    + task.getTask() + " / "
                    + task.getDeadline().format(dateTimeFormatter1) + "\n";
            buffer.write(content);
        } else if (command.startsWith("event")) {
            String content = "E / " + task.getTaskStatus() + " / "
                    + task.getTask() + " / "
                    + task.getEventStartTime().format(dateTimeFormatter1) + "-"
                    + task.getEventEndTime().format(dateTimeFormatter1) + "\n";
            buffer.write(content);
        } else if (command.startsWith("mark")) {
            String unchangedTasks = markOrUnmarkStorage(file, command, task, dateTimeFormatter1,
                    taskList, "[X]");
            file.createNewFile();
            file1 = new FileWriter(file);
            buffer = new BufferedWriter(file1);
            buffer.write(unchangedTasks);
        } else if (command.startsWith("unmark")) {
            String unchangedTasks = markOrUnmarkStorage(file, command, task, dateTimeFormatter1,
                    taskList, "[ ]");
            file.createNewFile();
            file1 = new FileWriter(file);
            buffer = new BufferedWriter(file1);
            buffer.write(unchangedTasks);
        } else if (command.startsWith("delete")) {
            String undeletedTasks = deleteStorage(file, command, taskList);
            file.createNewFile();
            file1 = new FileWriter(file);
            buffer = new BufferedWriter(file1);
            buffer.write(undeletedTasks);
        }
        buffer.close();
    }

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 (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 cs2103@comp.nus.edu.sg if you want to follow up on this post.