From 96d3388cd0e2b01534a9b1a4c5c20d4acf1e6ffd Mon Sep 17 00:00:00 2001 From: mingyuanc Date: Wed, 8 Nov 2023 16:01:23 +0800 Subject: [PATCH 1/5] Refactor and improve quality --- .../seedu/address/model/ITaskOperations.java | 15 ++-- .../seedu/address/model/ModelManager.java | 17 ++-- .../seedu/address/model/TaskOperation.java | 84 ++++++++++++++++--- 3 files changed, 90 insertions(+), 26 deletions(-) diff --git a/src/main/java/seedu/address/model/ITaskOperations.java b/src/main/java/seedu/address/model/ITaskOperations.java index 5fce749bbc2..0e84d8b0498 100644 --- a/src/main/java/seedu/address/model/ITaskOperations.java +++ b/src/main/java/seedu/address/model/ITaskOperations.java @@ -11,13 +11,6 @@ */ public interface ITaskOperations { - /** - * Checks if current task is present - * - * @param t The task in question - */ - boolean hasTask(Task t); - /** * Check if index is between 0 and task list size. */ @@ -36,6 +29,14 @@ public interface ITaskOperations { */ void addTask(Task t); + /** + * Checks if current task is present + * + * @param t The task in question + */ + boolean hasTask(Task t); + + /** * Deletes the task at the specified index * diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index 087ecc88fb4..0b8ce466ef4 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -52,7 +52,7 @@ public ModelManager(AbsolutePath currentPath, Root root, ReadOnlyUserPrefs userP * Constructs a model manager with all fields. */ public ModelManager(AbsolutePath currPath, Root root, ReadOnlyUserPrefs usePrefs, - AbsolutePath displayPath, boolean showTaskList) { + AbsolutePath displayPath, boolean showTaskList) { this(currPath, root, usePrefs); requireAllNonNull(displayPath, showTaskList); this.displayPath = displayPath; @@ -150,13 +150,6 @@ public boolean hasStudentWithId(StudentId id) { return false; } - @Override - public Group getGroupWithId(GroupId id) { - checkArgument(hasGroupWithId(id), - String.format(MESSAGE_INTERNAL_ERROR, "Group Id must exist in ProfBook")); - return this.root.getChild(id); - } - @Override public Student getStudentWithId(StudentId id) { checkArgument(hasStudentWithId(id), @@ -169,6 +162,14 @@ public Student getStudentWithId(StudentId id) { throw new IllegalArgumentException(String.format(MESSAGE_INTERNAL_ERROR, "Unexpected error occurred.")); } + @Override + public Group getGroupWithId(GroupId id) { + checkArgument(hasGroupWithId(id), + String.format(MESSAGE_INTERNAL_ERROR, "Group Id must exist in ProfBook")); + return this.root.getChild(id); + } + + @Override public boolean hasGroup(AbsolutePath path) { requireNonNull(path); diff --git a/src/main/java/seedu/address/model/TaskOperation.java b/src/main/java/seedu/address/model/TaskOperation.java index ec6aeac72bf..a9fed0d083d 100644 --- a/src/main/java/seedu/address/model/TaskOperation.java +++ b/src/main/java/seedu/address/model/TaskOperation.java @@ -10,50 +10,80 @@ import seedu.address.commons.util.JsonUtil; import seedu.address.model.task.ITaskListManager; import seedu.address.model.task.Task; +import seedu.address.model.task.exceptions.NoSuchTaskException; /** * Encapsulates the required logic for task operation */ public class TaskOperation implements ITaskOperations { + private final Logger logger = LogsCenter.getLogger(JsonUtil.class); private static final String MESSAGE_DUPLICATE_TASK = "Task must not exist in task list."; private static final String MESSAGE_TASK_NOT_FOUND = "Task not found in task list."; private final ITaskListManager baseDir; - private final Logger logger = LogsCenter.getLogger(JsonUtil.class); + /** + * Constructs a TaskOperation instance + * + * @param baseDir + */ public TaskOperation(ITaskListManager baseDir) { this.baseDir = baseDir; } - void stateLogger(String log) { + private void stateLogger(String log) { this.logger.info(log); } - void stateErrorLogger(String errMsg) { + private void stateErrorLogger(String errMsg) { this.logger.severe(errMsg); } + /** + * Checks if current task is present + * + * @param t The task in question + */ @Override - public int getTaskListSize() { - return this.baseDir.size(); + public boolean hasTask(Task t) { + return this.baseDir.contains(t); } + /** + * Adds a new tasks to the task list. + * Task must not be duplicated class. + * + * @param t The task in question + */ @Override - public boolean hasTask(Task t) { - return this.baseDir.contains(t); + public void addTask(Task task) { + checkArgument(!hasTask(task), MESSAGE_DUPLICATE_TASK); + this.baseDir.addTask(task); + this.stateLogger("Adding" + task.toString()); } + /** + * Check if index is between 0 and task list size. + */ @Override public boolean isValidIndex(int index) { return this.baseDir.isValidIndex(index); } + /** + * Return the size of the task list. + */ @Override - public void addTask(Task task) { - checkArgument(!hasTask(task), MESSAGE_DUPLICATE_TASK); - this.baseDir.addTask(task); - this.stateLogger("Adding" + task.toString()); + public int getTaskListSize() { + return this.baseDir.size(); } + /** + * Deletes the task at the specified index + * + * @param index - The index of the targeted class + * @return The deleted class + * @throws NoSuchTaskException if no task can be found by the index + */ @Override public Task deleteTask(int index) { checkArgument(isValidIndex(index), MESSAGE_TASK_NOT_FOUND); @@ -61,6 +91,13 @@ public Task deleteTask(int index) { return this.baseDir.deleteTask(index); } + /** + * Marks the task at the specified index as completed + * + * @param index - The index of the targeted class + * @return The marked task + * @throws NoSuchTaskException if no task can be found by the index + */ @Override public Task markTask(int index) { checkArgument(isValidIndex(index), MESSAGE_TASK_NOT_FOUND); @@ -68,6 +105,13 @@ public Task markTask(int index) { return this.baseDir.markTask(index); } + /** + * Marks the task at the specified index as not completed + * + * @param index - The index of the targeted class + * @return The un-marked task + * @throws NoSuchTaskException if no task can be found by the index + */ @Override public Task unmarkTask(int index) { checkArgument(isValidIndex(index), MESSAGE_TASK_NOT_FOUND); @@ -75,12 +119,25 @@ public Task unmarkTask(int index) { return this.baseDir.unmarkTask(index); } + /** + * Finds all matching task, compares by the task's description + * + * @param query - The String to match + * @return A list of all matching Tasks + */ @Override public List findTask(String query) { this.stateLogger("finding " + query); return this.baseDir.findTask(query); } + /** + * Returns the task at the specified index + * + * @param index - The index of the targeted class + * @return The specified task + * @throws NoSuchTaskException if no task can be found by the index + */ @Override public Task getTask(int index) { checkArgument(isValidIndex(index), MESSAGE_TASK_NOT_FOUND); @@ -88,6 +145,11 @@ public Task getTask(int index) { return this.baseDir.getTask(index); } + /** + * Returns all current task + * + * @return A list of all Tasks + */ @Override public List getAllTasks() { this.stateLogger("getting all "); From 87d101521aa89339c6b2dde8af6e737a31c0d84c Mon Sep 17 00:00:00 2001 From: mingyuanc Date: Wed, 8 Nov 2023 16:04:12 +0800 Subject: [PATCH 2/5] Refactor TaskOperation --- src/main/java/seedu/address/model/TaskOperation.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/main/java/seedu/address/model/TaskOperation.java b/src/main/java/seedu/address/model/TaskOperation.java index a9fed0d083d..e4908670f85 100644 --- a/src/main/java/seedu/address/model/TaskOperation.java +++ b/src/main/java/seedu/address/model/TaskOperation.java @@ -34,25 +34,21 @@ private void stateLogger(String log) { this.logger.info(log); } - private void stateErrorLogger(String errMsg) { - this.logger.severe(errMsg); - } - /** * Checks if current task is present * - * @param t The task in question + * @param task The task in question */ @Override - public boolean hasTask(Task t) { - return this.baseDir.contains(t); + public boolean hasTask(Task task) { + return this.baseDir.contains(task); } /** * Adds a new tasks to the task list. * Task must not be duplicated class. * - * @param t The task in question + * @param task The task in question */ @Override public void addTask(Task task) { From 4abbbf57ba3b7608fca13899afad6aeecb0d6bb6 Mon Sep 17 00:00:00 2001 From: mingyuanc Date: Wed, 8 Nov 2023 17:30:59 +0800 Subject: [PATCH 3/5] Refactor TaskOperation --- .../seedu/address/model/ChildOperation.java | 67 +++-- .../seedu/address/model/IChildOperation.java | 1 + ...askOperations.java => ITaskOperation.java} | 25 +- .../seedu/address/model/ModelManager.java | 237 +++++++++++------- .../seedu/address/model/TaskOperation.java | 8 +- .../seedu/address/model/profbook/Address.java | 2 +- ...ager.java => ChildAndTaskListManager.java} | 86 ++++--- ...ChildrenManager.java => ChildManager.java} | 36 +-- .../seedu/address/model/profbook/Email.java | 21 +- .../seedu/address/model/profbook/Group.java | 49 ++-- .../address/model/profbook/IChildElement.java | 11 +- ...hildrenManager.java => IChildManager.java} | 4 +- .../seedu/address/model/profbook/Name.java | 6 +- .../seedu/address/model/profbook/Phone.java | 1 - .../seedu/address/model/profbook/Root.java | 3 +- .../seedu/address/model/profbook/Student.java | 28 ++- 16 files changed, 355 insertions(+), 230 deletions(-) rename src/main/java/seedu/address/model/{ITaskOperations.java => ITaskOperation.java} (93%) rename src/main/java/seedu/address/model/profbook/{ChildrenAndTaskListManager.java => ChildAndTaskListManager.java} (55%) rename src/main/java/seedu/address/model/profbook/{ChildrenManager.java => ChildManager.java} (75%) rename src/main/java/seedu/address/model/profbook/{IChildrenManager.java => IChildManager.java} (95%) diff --git a/src/main/java/seedu/address/model/ChildOperation.java b/src/main/java/seedu/address/model/ChildOperation.java index 2ca194fec0e..af70d570f50 100644 --- a/src/main/java/seedu/address/model/ChildOperation.java +++ b/src/main/java/seedu/address/model/ChildOperation.java @@ -7,9 +7,9 @@ import seedu.address.commons.core.LogsCenter; import seedu.address.model.id.Id; -import seedu.address.model.profbook.ChildrenAndTaskListManager; +import seedu.address.model.profbook.ChildAndTaskListManager; import seedu.address.model.profbook.IChildElement; -import seedu.address.model.profbook.IChildrenManager; +import seedu.address.model.profbook.IChildManager; import seedu.address.model.profbook.exceptions.DuplicateChildException; import seedu.address.model.profbook.exceptions.NoSuchChildException; import seedu.address.model.task.ITaskListManager; @@ -22,14 +22,14 @@ */ public class ChildOperation> implements IChildOperation { public static final String MESSAGE_ALL_CHILDREN_MUST_BE_TASK_LIST_MANAGER = - "All children must be task list manager."; + "All children must be task list manager."; public static final String MESSAGE_INVALID_LEVEL = "Invalid level."; - private final IChildrenManager baseDir; + private final IChildManager baseDir; private final Logger logger = LogsCenter.getLogger(ChildOperation.class); - public ChildOperation(IChildrenManager baseDir) { + public ChildOperation(IChildManager baseDir) { this.baseDir = baseDir; } @@ -116,57 +116,68 @@ public int numOfChildren() { } @Override - public void addTaskToAllChildren(Task task, int level) { + public boolean checkIfAllChildrenHaveTask(Task task, int level) { List> children = getAllTaskListManagerChildrenAtLevel(level); for (IChildElement child : children) { - Task clonedTask = task.clone(); + + //Defensive programming - check if getAllTaskListManagerChildrenAtLevel works as expected if (!(child instanceof ITaskListManager)) { throw new IllegalArgumentException(MESSAGE_ALL_CHILDREN_MUST_BE_TASK_LIST_MANAGER); } - ITaskListManager tlm = (ITaskListManager) child; - if (tlm.contains(task)) { - continue; + // Type casting is safe as we checked earlier + ITaskListManager taskListManager = (ITaskListManager) child; + if (!taskListManager.contains(task)) { + return false; } - tlm.addTask(clonedTask); } + return true; } @Override - public boolean checkIfAllChildrenHaveTask(Task task, int level) { + public boolean checkIfAnyChildHasTask(Task task, int level) { List> children = getAllTaskListManagerChildrenAtLevel(level); for (IChildElement child : children) { + + //Defensive programming - check if getAllTaskListManagerChildrenAtLevel works as expected if (!(child instanceof ITaskListManager)) { throw new IllegalArgumentException(MESSAGE_ALL_CHILDREN_MUST_BE_TASK_LIST_MANAGER); } - ITaskListManager tlm = (ITaskListManager) child; - if (!tlm.contains(task)) { - return false; + + // Type casting is safe as we checked earlier + ITaskListManager taskListManager = (ITaskListManager) child; + if (taskListManager.contains(task)) { + return true; } } - return true; + return false; } @Override - public boolean checkIfAnyChildHasTask(Task task, int level) { + public void addTaskToAllChildren(Task task, int level) { List> children = getAllTaskListManagerChildrenAtLevel(level); for (IChildElement child : children) { + Task clonedTask = task.clone(); + + //Defensive programming - check if getAllTaskListManagerChildrenAtLevel works as expected if (!(child instanceof ITaskListManager)) { throw new IllegalArgumentException(MESSAGE_ALL_CHILDREN_MUST_BE_TASK_LIST_MANAGER); } - ITaskListManager tlm = (ITaskListManager) child; - if (tlm.contains(task)) { - return true; + + // Type casting is safe as we checked earlier + ITaskListManager taskListManager = (ITaskListManager) child; + if (taskListManager.contains(task)) { + continue; } + taskListManager.addTask(clonedTask); } - - return false; } + @Override public boolean equals(Object o) { if (this == o) { @@ -189,11 +200,13 @@ private List> getAllTaskListManagerChildrenAtLevel(int level) { while (--level > 0) { List> list = new ArrayList<>(); for (IChildElement child : children) { - if (child instanceof ChildrenAndTaskListManager) { - ChildrenAndTaskListManager ctlm = (ChildrenAndTaskListManager) child; - list.addAll(ctlm.getAllChildren()); - } else { - throw new IllegalArgumentException(MESSAGE_INVALID_LEVEL); + + if (child instanceof ChildAndTaskListManager) { // If child is a group directory + ChildAndTaskListManager childrenAndTaskListManager = + (ChildAndTaskListManager) child; // type casting is safe as we checked earlier + list.addAll(childrenAndTaskListManager.getAllChildren()); + } else { // If child is a student directory + throw new IllegalArgumentException(MESSAGE_INVALID_LEVEL); // Student does not have any child } } children = new ArrayList<>(list); diff --git a/src/main/java/seedu/address/model/IChildOperation.java b/src/main/java/seedu/address/model/IChildOperation.java index eb46aaa1ac0..97ed550c233 100644 --- a/src/main/java/seedu/address/model/IChildOperation.java +++ b/src/main/java/seedu/address/model/IChildOperation.java @@ -16,6 +16,7 @@ * @param Type of the child */ public interface IChildOperation> { + /** * Adds the child to list of children * diff --git a/src/main/java/seedu/address/model/ITaskOperations.java b/src/main/java/seedu/address/model/ITaskOperation.java similarity index 93% rename from src/main/java/seedu/address/model/ITaskOperations.java rename to src/main/java/seedu/address/model/ITaskOperation.java index 0e84d8b0498..aed6d3b0488 100644 --- a/src/main/java/seedu/address/model/ITaskOperations.java +++ b/src/main/java/seedu/address/model/ITaskOperation.java @@ -9,26 +9,19 @@ * Interface for classes that operations that involve a task list, ensures that all basic functions are present to * interact with TaskListManager instance */ -public interface ITaskOperations { - - /** - * Check if index is between 0 and task list size. - */ - boolean isValidIndex(int index); - - /** - * Return the size of the task list. - */ - int getTaskListSize(); - +public interface ITaskOperation { /** - * Adds a new tasks to the task list. - * Task must not be duplicated class. + * Adds the tasks to the task list granted it does not result in a duplicate * * @param t The task in question */ void addTask(Task t); + /** + * Check if index is between 0 and task list size. + */ + boolean isValidIndex(int index); + /** * Checks if current task is present * @@ -36,6 +29,10 @@ public interface ITaskOperations { */ boolean hasTask(Task t); + /** + * Return the current size of the task list. + */ + int getTaskListSize(); /** * Deletes the task at the specified index diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index b6e14ba1cb1..9603fec5c96 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -27,35 +27,35 @@ * Represents the in-memory model of the ProfBook data. */ public class ModelManager implements Model { - public static final String MESSAGE_PATH_NOT_FOUND = "Path must exist in ProfBook"; - public static final String MESSAGE_STUDENT_NOT_FOUND = "Student must exist in ProfBook"; - public static final String MESSAGE_GROUP_NOT_FOUND = "Group must exist in ProfBook"; - public static final String MESSAGE_STUDENT_ID_NOT_FOUND = "Student Id must exist in ProfBook"; - public static final String MESSAGE_GROUP_ID_NOT_FOUND = "Group Id must exist in ProfBook"; - public static final String MESSAGE_GROUP_INFO_NOT_FOUND = "Path must have group information"; - public static final String MESSAGE_REQUIRE_STUDENT_PATH = "Path must be student directory"; - public static final String MESSAGE_REQUIRE_CHILDREN_MANAGER_PATH = "Path must be children manager"; - public static final String MESSAGE_REQUIRE_TASK_LIST_MANAGER_PATH = "Path must be task list manager"; - public static final String MESSAGE_UNEXPECTED_ERROR = "Unexpected error occurred."; - public static final String MESSAGE_STUDENT_PATH_NOT_NAVIGABLE = "Student path is not navigable"; - + public static final String MESSAGE_GROUP_INFO_NOT_FOUND = "Path must have group information."; + public static final String MESSAGE_REQUIRE_CHILDREN_MANAGER_PATH = "Path must be children manager."; + public static final String MESSAGE_GROUP_ID_NOT_FOUND = "Group Id must exist in ProfBook."; + public static final String MESSAGE_STUDENT_PATH_NOT_NAVIGABLE = "Student path is not navigable."; + public static final String MESSAGE_UNEXPECTED_ERROR = "An unexpected error occurred."; + public static final String MESSAGE_REQUIRE_TASK_LIST_MANAGER_PATH = "Path must be task list manager."; + public static final String MESSAGE_REQUIRE_STUDENT_PATH = "Path must be student directory."; + public static final String MESSAGE_PATH_NOT_FOUND = "Path must exist in ProfBook."; + public static final String MESSAGE_STUDENT_NOT_FOUND = "Student must exist in ProfBook."; + public static final String MESSAGE_STUDENT_ID_NOT_FOUND = "Student Id must exist in ProfBook."; + public static final String MESSAGE_GROUP_NOT_FOUND = "Group must exist in ProfBook."; private static final Logger logger = LogsCenter.getLogger(Model.class); + private final ObservableList displayList = FXCollections.observableArrayList(); + private final UserPrefs userPrefs; private Root root; - private AbsolutePath currentPath; private boolean showTaskList = false; + private AbsolutePath currentPath; private AbsolutePath displayPath; /** - * Construct a model manager with current path, root (ProfBook) and userPrefs. + * Constructs a new model manager with no data. */ - public ModelManager(AbsolutePath currentPath, Root root, ReadOnlyUserPrefs userPrefs) { - requireAllNonNull(currentPath, root, userPrefs); - this.currentPath = currentPath; - this.displayPath = currentPath; - this.root = new Root(root); - this.userPrefs = new UserPrefs(userPrefs); + public ModelManager() { + this.currentPath = AbsolutePath.ROOT_PATH; + this.displayPath = AbsolutePath.ROOT_PATH; + this.root = new Root(); + this.userPrefs = new UserPrefs(); updateList(); } @@ -72,48 +72,81 @@ public ModelManager(AbsolutePath currPath, Root root, ReadOnlyUserPrefs usePrefs } /** - * Constructs a new model manager with empty data. + * Construct a model manager with only current path, root (ProfBook) and userPrefs. */ - public ModelManager() { - this.currentPath = AbsolutePath.ROOT_PATH; - this.displayPath = AbsolutePath.ROOT_PATH; - this.root = new Root(); - this.userPrefs = new UserPrefs(); + public ModelManager(AbsolutePath currentPath, Root root, ReadOnlyUserPrefs userPrefs) { + requireAllNonNull(currentPath, root, userPrefs); + this.userPrefs = new UserPrefs(userPrefs); + this.displayPath = currentPath; + this.currentPath = currentPath; + this.root = new Root(root); updateList(); } - //=========== UserPrefs ================================================================================== - public void setUserPrefs(ReadOnlyUserPrefs userPrefs) { - requireNonNull(userPrefs); - this.userPrefs.resetData(userPrefs); - } - public ReadOnlyUserPrefs getUserPrefs() { - return userPrefs; - } + //=========== UserPrefs =================================================================================== - public GuiSettings getGuiSettings() { - return userPrefs.getGuiSettings(); + /** + * Updates the current storage file path + * + * @param addressBookFilePath The new Storage file Path + */ + public void setProfBookFilePath(Path addressBookFilePath) { + requireNonNull(addressBookFilePath); + logger.info("Updating storage file path"); + userPrefs.setAddressBookFilePath(addressBookFilePath); } + /** + * Updates the current GUI preference with a new instance + * + * @param guiSettings the updated GUI preference + */ public void setGuiSettings(GuiSettings guiSettings) { requireNonNull(guiSettings); + logger.info("Updating GUI preference"); userPrefs.setGuiSettings(guiSettings); } + /** + * Gets the current storage file path + * + * @return The path instance + */ public Path getProfBookFilePath() { return userPrefs.getProfBookFilePath(); } - public void setProfBookFilePath(Path addressBookFilePath) { - requireNonNull(addressBookFilePath); - userPrefs.setAddressBookFilePath(addressBookFilePath); + /** + * Updates the current user preference with a new instance + * + * @param userPrefs the updated user preference + */ + public void setUserPrefs(ReadOnlyUserPrefs userPrefs) { + requireNonNull(userPrefs); + logger.info("Updating user preference"); + this.userPrefs.resetData(userPrefs); + } + + /** + * Gets the user preference + */ + public ReadOnlyUserPrefs getUserPrefs() { + return this.userPrefs; + } + + /** + * Gets the GUI settings from user preference + */ + public GuiSettings getGuiSettings() { + return this.userPrefs.getGuiSettings(); } //=========== ProfBook Model ================================================================================ @Override public void setRoot(Root root) { requireNonNull(root); + logger.info("Resetting to root directory"); this.root = root; this.currentPath = AbsolutePath.ROOT_PATH; this.displayPath = AbsolutePath.ROOT_PATH; @@ -122,13 +155,24 @@ public void setRoot(Root root) { } @Override - public AbsolutePath getCurrPath() { - return this.currentPath; + public boolean hasStudentWithId(StudentId id) { + logger.info("finding student with id: " + id); + for (Group group : this.root.getAllChildren()) { + if (group.hasChild(id)) { + return true; + } + } + return false; } @Override - public AbsolutePath getDisplayPath() { - return this.displayPath; + public boolean hasChildrenListInCurrentPath() { + return hasChildrenListInPath(currentPath); + } + + @Override + public boolean hasTaskListInCurrentPath() { + return hasTaskListInPath(currentPath); } @Override @@ -137,13 +181,13 @@ public boolean isShowTaskList() { } @Override - public boolean hasTaskListInCurrentPath() { - return hasTaskListInPath(currentPath); + public AbsolutePath getDisplayPath() { + return this.displayPath; } @Override - public boolean hasChildrenListInCurrentPath() { - return hasChildrenListInPath(currentPath); + public AbsolutePath getCurrPath() { + return this.currentPath; } @Override @@ -151,16 +195,6 @@ public boolean hasGroupWithId(GroupId id) { return this.root.hasChild(id); } - @Override - public boolean hasStudentWithId(StudentId id) { - for (Group group : this.root.getAllChildren()) { - if (group.hasChild(id)) { - return true; - } - } - return false; - } - @Override public Group getGroupWithId(GroupId id) { checkArgument(hasGroupWithId(id), @@ -172,27 +206,30 @@ public Group getGroupWithId(GroupId id) { public Student getStudentWithId(StudentId id) { checkArgument(hasStudentWithId(id), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_STUDENT_ID_NOT_FOUND)); + logger.info("Finding student with id: " + id); for (Group group : this.root.getAllChildren()) { if (group.hasChild(id)) { return group.getChild(id); } } + logger.severe("Unable to find student with id: " + id); throw new IllegalArgumentException(String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_UNEXPECTED_ERROR)); } - @Override - public Group getGroupWithId(GroupId id) { - checkArgument(hasGroupWithId(id), - String.format(MESSAGE_INTERNAL_ERROR, "Group Id must exist in ProfBook")); - return this.root.getChild(id); - } - @Override public boolean hasGroup(AbsolutePath path) { requireNonNull(path); checkArgument(!path.isRootDirectory(), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_GROUP_INFO_NOT_FOUND)); + logger.info("Finding group at " + path); + + // defensive programming + if (path.getGroupId().isEmpty()) { + logger.severe("Invalid path: " + path); + throw new IllegalArgumentException(String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_UNEXPECTED_ERROR)); + } + GroupId grpId = path.getGroupId().get(); return root.hasChild(grpId); } @@ -202,20 +239,28 @@ public boolean hasStudent(AbsolutePath path) { requireNonNull(path); checkArgument(path.isStudentDirectory(), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_REQUIRE_STUDENT_PATH)); + logger.info("Finding student at " + path); if (!hasGroup(path)) { return false; } + // defensive programming + if (path.getStudentId().isEmpty()) { + logger.severe("Invalid path: " + path); + throw new IllegalArgumentException(String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_UNEXPECTED_ERROR)); + } + StudentId stuId = path.getStudentId().get(); Group grp = getGroupFromPath(path); - return grp.hasChild(stuId); } @Override public boolean hasPath(AbsolutePath path) { requireNonNull(path); + logger.info("Checking if path is present, path: " + path); + if (path.isRootDirectory()) { return true; } @@ -234,8 +279,8 @@ public void changeDirectory(AbsolutePath path) { String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_PATH_NOT_FOUND)); checkArgument(!path.isStudentDirectory(), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_STUDENT_PATH_NOT_NAVIGABLE)); - currentPath = path; - displayPath = path; + this.currentPath = path; + this.displayPath = path; logger.fine("Change directory to " + currentPath); showChildrenList(); } @@ -248,19 +293,21 @@ public ObservableList getDisplayList() { @Override public void updateList() { + logger.info("Updating display list"); List temp = new ArrayList<>(); - if (showTaskList) { - TaskOperation taskOperation = taskOperation(displayPath); + + if (this.showTaskList) { // If showing task list should get all current tasks + TaskOperation taskOperation = taskOperation(this.displayPath); temp = new ArrayList<>(taskOperation.getAllTasks()); - } else if (displayPath.isRootDirectory()) { + } else if (this.displayPath.isRootDirectory()) { // If showing root, get all groups under root ChildOperation childOperation = rootChildOperation(); temp = new ArrayList<>(childOperation.getAllChildren()); - } else if (displayPath.isGroupDirectory()) { - ChildOperation childOperation = groupChildOperation(displayPath); + } else if (this.displayPath.isGroupDirectory()) { // If showing group, get all student under root + ChildOperation childOperation = groupChildOperation(this.displayPath); temp = new ArrayList<>(childOperation.getAllChildren()); } - displayList.clear(); - displayList.setAll(temp); + this.displayList.clear(); + this.displayList.setAll(temp); } @Override @@ -268,39 +315,43 @@ public void setDisplayPath(AbsolutePath path) { requireNonNull(path); checkArgument(hasPath(path), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_PATH_NOT_FOUND)); - displayPath = path; + + logger.info("Changing display path"); + this.displayPath = path; } @Override public boolean hasTaskListInDisplayPath() { - return hasTaskListInPath(displayPath); + return hasTaskListInPath(this.displayPath); } @Override - public boolean hasChildrenListInDisplayPath() { - return hasChildrenListInPath(displayPath); + public void showTaskList() { + checkArgument(hasTaskListInDisplayPath(), + String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_REQUIRE_TASK_LIST_MANAGER_PATH)); + this.showTaskList = true; + updateList(); } @Override public void showChildrenList() { checkArgument(hasChildrenListInDisplayPath(), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_REQUIRE_CHILDREN_MANAGER_PATH)); - showTaskList = false; + this.showTaskList = false; updateList(); } @Override - public void showTaskList() { - checkArgument(hasTaskListInDisplayPath(), - String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_REQUIRE_TASK_LIST_MANAGER_PATH)); - showTaskList = true; - updateList(); + public boolean hasChildrenListInDisplayPath() { + return hasChildrenListInPath(this.displayPath); } + //=========== Model Management Operation ============================================================= @Override public ChildOperation rootChildOperation() { - return new ChildOperation<>(root); + logger.info("New GroupChildOperation at root"); + return new ChildOperation<>(this.root); } @Override @@ -310,6 +361,7 @@ public ChildOperation groupChildOperation(AbsolutePath path) { String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_GROUP_INFO_NOT_FOUND)); checkArgument(hasGroup(path), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_GROUP_NOT_FOUND)); + logger.info("New GroupChildOperation at group: " + path); return new ChildOperation<>(getGroupFromPath(path)); } @@ -320,7 +372,7 @@ public TaskOperation taskOperation(AbsolutePath path) { String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_REQUIRE_TASK_LIST_MANAGER_PATH)); checkArgument(hasPath(path), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_PATH_NOT_FOUND)); - + logger.info("New TaskOperation"); if (path.isGroupDirectory()) { return new TaskOperation(getGroupFromPath(path)); } @@ -353,6 +405,12 @@ private Group getGroupFromPath(AbsolutePath path) { checkArgument(hasGroup(path), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_GROUP_NOT_FOUND)); + // defensive programming + if (path.getGroupId().isEmpty()) { + logger.severe("Invalid path: " + path); + throw new IllegalArgumentException(String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_UNEXPECTED_ERROR)); + } + GroupId grpId = path.getGroupId().get(); return root.getChild(grpId); @@ -369,6 +427,12 @@ private Student getStudentFromPath(AbsolutePath path) { checkArgument(hasStudent(path), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_STUDENT_NOT_FOUND)); + // defensive programming + if (path.getGroupId().isEmpty() || path.getStudentId().isEmpty()) { + logger.severe("Invalid path: " + path); + throw new IllegalArgumentException(String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_UNEXPECTED_ERROR)); + } + GroupId grpId = path.getGroupId().get(); StudentId stuId = path.getStudentId().get(); @@ -413,5 +477,4 @@ public String toString() { .add("userPrefs", userPrefs) .toString(); } - } diff --git a/src/main/java/seedu/address/model/TaskOperation.java b/src/main/java/seedu/address/model/TaskOperation.java index e4908670f85..65e60cecbbc 100644 --- a/src/main/java/seedu/address/model/TaskOperation.java +++ b/src/main/java/seedu/address/model/TaskOperation.java @@ -15,16 +15,18 @@ /** * Encapsulates the required logic for task operation */ -public class TaskOperation implements ITaskOperations { - private final Logger logger = LogsCenter.getLogger(JsonUtil.class); +public class TaskOperation implements ITaskOperation { + private static final String MESSAGE_DUPLICATE_TASK = "Task must not exist in task list."; private static final String MESSAGE_TASK_NOT_FOUND = "Task not found in task list."; + private final Logger logger = LogsCenter.getLogger(JsonUtil.class); + private final ITaskListManager baseDir; /** * Constructs a TaskOperation instance * - * @param baseDir + * @param baseDir - The current directory to enact task operations */ public TaskOperation(ITaskListManager baseDir) { this.baseDir = baseDir; diff --git a/src/main/java/seedu/address/model/profbook/Address.java b/src/main/java/seedu/address/model/profbook/Address.java index c84cdbbc36f..3c8247fe258 100644 --- a/src/main/java/seedu/address/model/profbook/Address.java +++ b/src/main/java/seedu/address/model/profbook/Address.java @@ -11,7 +11,7 @@ public class Address { public static final String MESSAGE_CONSTRAINTS = "Addresses can take any values, and it should not be blank"; public static final Address PLACEHOLDER = new Address("n/a"); - /* + /** * The first character of the address must not be a whitespace, * otherwise " " (a blank string) becomes a valid input. */ diff --git a/src/main/java/seedu/address/model/profbook/ChildrenAndTaskListManager.java b/src/main/java/seedu/address/model/profbook/ChildAndTaskListManager.java similarity index 55% rename from src/main/java/seedu/address/model/profbook/ChildrenAndTaskListManager.java rename to src/main/java/seedu/address/model/profbook/ChildAndTaskListManager.java index 79021bc7640..8365ff1c7a4 100644 --- a/src/main/java/seedu/address/model/profbook/ChildrenAndTaskListManager.java +++ b/src/main/java/seedu/address/model/profbook/ChildAndTaskListManager.java @@ -16,39 +16,43 @@ /** * A child element that is both children and task list Manager. */ -public abstract class ChildrenAndTaskListManager> - implements IChildElement, IChildrenManager, ITaskListManager { - private ChildrenManager childrenManager; - private TaskListManager taskListManager; +public abstract class ChildAndTaskListManager> + implements IChildElement, IChildManager, ITaskListManager { + private final ChildManager childrenManager; + private final TaskListManager taskListManager; /** - * Constructs a new {@code ChildrenAndTaskListManager}. + * Constructs a {@code ChildAndTaskListManager} with the data in {@code toBeCopied}. + * + * @param toBeCopied - data from storage */ - public ChildrenAndTaskListManager() { - childrenManager = new ChildrenManager<>(); - taskListManager = new TaskListManager(); + public ChildAndTaskListManager(ChildAndTaskListManager toBeCopied) { + this.childrenManager = new ChildManager<>(toBeCopied.childrenManager); + this.taskListManager = new TaskListManager(toBeCopied.taskListManager); } /** - * Constructs a new {@code ChildrenAndTaskListManager} with given children and tasklist. + * Constructs a new {@code ChildAndTaskListManager} with given children and task list. + * + * @param children - Map of current children this directory have + * @param taskList - Arraylist of tasks assigned to this directory */ - public ChildrenAndTaskListManager(Map children, ReadOnlyTaskList taskList) { - childrenManager = new ChildrenManager<>(children); + public ChildAndTaskListManager(Map children, ReadOnlyTaskList taskList) { + childrenManager = new ChildManager<>(children); taskListManager = new TaskListManager(taskList.getAllTasks()); } /** - * Construcst a {@code ChildrenAndTaskListManager} with the data in {@code toBeCopied}. + * Constructs a new {@code ChildAndTaskListManager}. */ - public ChildrenAndTaskListManager(ChildrenAndTaskListManager toBeCopied) { - this.childrenManager = new ChildrenManager<>(toBeCopied.childrenManager); - this.taskListManager = new TaskListManager(toBeCopied.taskListManager); - } - - public ChildrenManager getChildrenManger() { - return childrenManager; + public ChildAndTaskListManager() { + this.childrenManager = new ChildManager<>(); + this.taskListManager = new TaskListManager(); } + /** + * Returns the current task list Manager + */ public TaskListManager getTaskListManager() { return taskListManager; } @@ -57,101 +61,101 @@ public TaskListManager getTaskListManager() { @Override public void addChild(Id id, T child) throws DuplicateChildException { - childrenManager.addChild(id, child); + this.childrenManager.addChild(id, child); } @Override public T deleteChild(Id id) throws NoSuchChildException { - return childrenManager.deleteChild(id); + return this.childrenManager.deleteChild(id); } @Override public boolean hasChild(Id id) { - return childrenManager.hasChild(id); + return this.childrenManager.hasChild(id); } @Override public T getChild(Id id) throws NoSuchChildException { - return childrenManager.getChild(id); + return this.childrenManager.getChild(id); } @Override public int numOfChildren() { - return childrenManager.numOfChildren(); + return this.childrenManager.numOfChildren(); } @Override public List getAllChildren() { - return childrenManager.getAllChildren(); + return this.childrenManager.getAllChildren(); } @Override public Map getChildren() { - return childrenManager.getChildren(); + return this.childrenManager.getChildren(); } //=========== TaskList Manager ================================================================================== @Override public void addTask(Task t) { - taskListManager.addTask(t); + this.taskListManager.addTask(t); } @Override public Task deleteTask(int index) throws NoSuchTaskException { - return taskListManager.deleteTask(index); + return this.taskListManager.deleteTask(index); } @Override public Task markTask(int index) throws NoSuchTaskException { - return taskListManager.markTask(index); + return this.taskListManager.markTask(index); } @Override public Task unmarkTask(int index) throws NoSuchTaskException { - return taskListManager.unmarkTask(index); + return this.taskListManager.unmarkTask(index); } @Override public List findTask(String query) throws NoSuchTaskException { - return taskListManager.findTask(query); + return this.taskListManager.findTask(query); } @Override public boolean isValidIndex(int index) { - return taskListManager.isValidIndex(index); + return this.taskListManager.isValidIndex(index); } @Override public int size() { - return taskListManager.size(); + return this.taskListManager.size(); } @Override public boolean isEmpty() { - return taskListManager.isEmpty(); + return this.taskListManager.isEmpty(); } @Override public Task getTask(int index) throws NoSuchTaskException { - return taskListManager.getTask(index); + return this.taskListManager.getTask(index); } @Override public boolean contains(Task t) { - return taskListManager.contains(t); + return this.taskListManager.contains(t); } @Override public List getAllTasks() { - return taskListManager.getAllTasks(); + return this.taskListManager.getAllTasks(); } @Override public String toString() { return new ToStringBuilder(this) - .add("Task List", taskListManager) - .add("Children List", childrenManager) + .add("Task List", this.taskListManager) + .add("Children List", this.childrenManager) .toString(); } @@ -162,11 +166,11 @@ public boolean equals(Object other) { } // instanceof handles nulls - if (!(other instanceof ChildrenAndTaskListManager)) { + if (!(other instanceof ChildAndTaskListManager)) { return false; } - ChildrenAndTaskListManager otherChildrenAndTaskListManager = (ChildrenAndTaskListManager) other; + ChildAndTaskListManager otherChildrenAndTaskListManager = (ChildAndTaskListManager) other; return this.childrenManager.equals(otherChildrenAndTaskListManager.childrenManager) && this.taskListManager.equals(otherChildrenAndTaskListManager.taskListManager); } diff --git a/src/main/java/seedu/address/model/profbook/ChildrenManager.java b/src/main/java/seedu/address/model/profbook/ChildManager.java similarity index 75% rename from src/main/java/seedu/address/model/profbook/ChildrenManager.java rename to src/main/java/seedu/address/model/profbook/ChildManager.java index e4f1dcadd6b..8936e36679e 100644 --- a/src/main/java/seedu/address/model/profbook/ChildrenManager.java +++ b/src/main/java/seedu/address/model/profbook/ChildManager.java @@ -5,7 +5,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; @@ -19,48 +18,54 @@ * * @param to represent the children type, as of v1.2 only student and group */ -public class ChildrenManager> implements IChildrenManager { +public class ChildManager> implements IChildManager { + /** * Maps the id to the children */ private final Map children; /** - * Construct a children manager with given task list and children map. + * Constructs a children manager with given task list and children map. + * + * @param children - The map of the directory's current child */ - public ChildrenManager(Map children) { + public ChildManager(Map children) { requireAllNonNull(children); Map tempMap = new HashMap<>(); Id key; - Iterator it = children.keySet().iterator(); - while (it.hasNext()) { - key = it.next(); + for (Id id : children.keySet()) { + key = id; tempMap.put(key, children.get(key).deepCopy()); } this.children = tempMap; } /** - * Construct a new children manager. + * Constructs a new children manager. */ - public ChildrenManager() { + public ChildManager() { children = new HashMap<>(); } /** - * Constructs a {@code ChildrenManager} with the data in {@code toBeCopied}. - * @param toBeCopied + * Constructs a {@code ChildManager} with the data in {@code toBeCopied}. + * + * @param toBeCopied - Data extracted from storage */ - public ChildrenManager(ChildrenManager toBeCopied) { + public ChildManager(ChildManager toBeCopied) { this(toBeCopied.children); } @Override public void addChild(Id id, T child) throws DuplicateChildException { T currChild = this.children.get(id); + + // Defensive programming if (currChild != null) { throw new DuplicateChildException(id.toString()); } + this.children.put(id, child); } @@ -79,6 +84,7 @@ public boolean hasChild(Id id) { @Override public T getChild(Id id) throws NoSuchChildException { T child = this.children.get(id); + // Defensive programming if (child == null) { throw new NoSuchChildException(id.toString()); } @@ -93,7 +99,7 @@ public int numOfChildren() { @Override public List getAllChildren() { List childrenList = new ArrayList<>(this.children.values()); - Collections.sort(childrenList); + Collections.sort(childrenList); // Important to sort as Map stores them randomly return childrenList; } @@ -117,11 +123,11 @@ public boolean equals(Object other) { } // instanceof handles nulls - if (!(other instanceof ChildrenManager)) { + if (!(other instanceof ChildManager)) { return false; } - ChildrenManager otherChildrenManger = (ChildrenManager) other; + ChildManager otherChildrenManger = (ChildManager) other; return this.children.equals(otherChildrenManger.children); } } diff --git a/src/main/java/seedu/address/model/profbook/Email.java b/src/main/java/seedu/address/model/profbook/Email.java index b2e30b46a1b..ef705c92105 100644 --- a/src/main/java/seedu/address/model/profbook/Email.java +++ b/src/main/java/seedu/address/model/profbook/Email.java @@ -10,17 +10,18 @@ public class Email { public static final Email PLACEHOLDER = new Email("n/a"); private static final String SPECIAL_CHARACTERS = "+_.-"; + public static final String MESSAGE_CONSTRAINTS = "Emails should be of the format local-part@domain " - + "and adhere to the following constraints:\n" - + "1. The local-part should only contain alphanumeric characters and these special characters, excluding " - + "the parentheses, (" + SPECIAL_CHARACTERS + "). The local-part may not start or end with any special " - + "characters.\n" - + "2. This is followed by a '@' and then a domain name. The domain name is made up of domain labels " - + "separated by periods.\n" - + "The domain name must:\n" - + " - end with a domain label at least 2 characters long\n" - + " - have each domain label start and end with alphanumeric characters\n" - + " - have each domain label consist of alphanumeric characters, separated only by hyphens, if any."; + + "and adhere to the following constraints:\n" + + "1. The local-part should only contain alphanumeric characters and these special characters, excluding " + + "the parentheses, (" + SPECIAL_CHARACTERS + "). The local-part may not start or end with any special " + + "characters.\n" + + "2. This is followed by a '@' and then a domain name. The domain name is made up of domain labels " + + "separated by periods.\n" + + "The domain name must:\n" + + " - end with a domain label at least 2 characters long\n" + + " - have each domain label start and end with alphanumeric characters\n" + + " - have each domain label consist of alphanumeric characters, separated only by hyphens, if any."; // alphanumeric and special characters private static final String ALPHANUMERIC_NO_UNDERSCORE = "[^\\W_]+"; // alphanumeric characters except underscore private static final String LOCAL_PART_REGEX = "^" + ALPHANUMERIC_NO_UNDERSCORE + "([" + SPECIAL_CHARACTERS + "]" diff --git a/src/main/java/seedu/address/model/profbook/Group.java b/src/main/java/seedu/address/model/profbook/Group.java index ae6e584ff41..84b00742092 100644 --- a/src/main/java/seedu/address/model/profbook/Group.java +++ b/src/main/java/seedu/address/model/profbook/Group.java @@ -13,7 +13,7 @@ /** * Encapsulates logic for a group within a tutorial group */ -public class Group extends ChildrenAndTaskListManager { +public class Group extends ChildAndTaskListManager { /** * Name of the group @@ -25,23 +25,11 @@ public class Group extends ChildrenAndTaskListManager { */ private final GroupId id; - /** - * Constructs a Group instance with all fields. - * - * @param taskList - The task list associated with this group - * @param students - The list of students in this group - * @param name - The group name - * @param id - Unique identifier of the group - */ - public Group(ReadOnlyTaskList taskList, Map students, Name name, GroupId id) { - super(students, taskList); - requireAllNonNull(name, id); - this.name = name; - this.id = id; - } - /** * Constructs a new {@code Group} without task list and student map. + * + * @param name - Name of the group + * @param id - Unique id of the group */ public Group(Name name, GroupId id) { super(); @@ -49,8 +37,11 @@ public Group(Name name, GroupId id) { this.id = id; } + /** - * Create a {@code Group} with the data in {@code toBeCopied}. + * Creates a {@code Group} with the data in {@code toBeCopied}. + * + * @param toBeCopied - Data extracted from storage */ public Group(Group toBeCopied) { super(toBeCopied); @@ -58,10 +49,31 @@ public Group(Group toBeCopied) { this.id = toBeCopied.id; } + /** + * Constructs a Group instance with all fields. + * + * @param taskList - The task list associated with this group + * @param students - The list of students in this group + * @param name - The group name + * @param id - Unique identifier of the group + */ + public Group(ReadOnlyTaskList taskList, Map students, Name name, GroupId id) { + super(students, taskList); + requireAllNonNull(name, id); + this.name = name; + this.id = id; + } + + /** + * Returns the group's id + */ public GroupId getId() { return id; } + /** + * Returns the group's name + */ public Name getName() { return name; } @@ -85,7 +97,8 @@ public String toString() { .toString(); } - @Override public int compareTo(Group other) { + @Override + public int compareTo(Group other) { if (this.id.toString().compareTo(other.id.toString()) != 0) { return this.id.toString().compareTo(other.id.toString()); } else { diff --git a/src/main/java/seedu/address/model/profbook/IChildElement.java b/src/main/java/seedu/address/model/profbook/IChildElement.java index 3e1294239b9..ed648064112 100644 --- a/src/main/java/seedu/address/model/profbook/IChildElement.java +++ b/src/main/java/seedu/address/model/profbook/IChildElement.java @@ -9,14 +9,15 @@ public interface IChildElement extends Displayable, Comparable { /** - * Creates a clone of the current element, this is to achieve immutability. + * Returns the unique id of the child. + */ + Id getId(); + + /** + * Creates a deep copy of the current element, this is to achieve immutability. * * @return The clone of the IChildElement */ T deepCopy(); - /** - * Returns the unique id. - */ - Id getId(); } diff --git a/src/main/java/seedu/address/model/profbook/IChildrenManager.java b/src/main/java/seedu/address/model/profbook/IChildManager.java similarity index 95% rename from src/main/java/seedu/address/model/profbook/IChildrenManager.java rename to src/main/java/seedu/address/model/profbook/IChildManager.java index 1a798da1108..6e023d86681 100644 --- a/src/main/java/seedu/address/model/profbook/IChildrenManager.java +++ b/src/main/java/seedu/address/model/profbook/IChildManager.java @@ -10,7 +10,8 @@ /** * API for Children Manager */ -public interface IChildrenManager> { +public interface IChildManager> { + /** * Adds the child to list of children * @@ -58,7 +59,6 @@ public interface IChildrenManager> { /** * Returns children map. - * */ Map getChildren(); } diff --git a/src/main/java/seedu/address/model/profbook/Name.java b/src/main/java/seedu/address/model/profbook/Name.java index 294517b17d9..4e0c8de8a5d 100644 --- a/src/main/java/seedu/address/model/profbook/Name.java +++ b/src/main/java/seedu/address/model/profbook/Name.java @@ -12,7 +12,7 @@ public class Name { public static final String MESSAGE_CONSTRAINTS = "Names should only contain alphanumeric characters and spaces, and it should not be blank"; - /* + /** * The first character of the address must not be a whitespace, * otherwise " " (a blank string) becomes a valid input. */ @@ -56,8 +56,8 @@ public boolean equals(Object other) { } Name otherName = (Name) other; - return fullName.toLowerCase() - .equals(otherName.fullName.toLowerCase()); + return fullName + .equalsIgnoreCase(otherName.fullName); } @Override diff --git a/src/main/java/seedu/address/model/profbook/Phone.java b/src/main/java/seedu/address/model/profbook/Phone.java index 15d3f9dba4d..2c2621866ff 100644 --- a/src/main/java/seedu/address/model/profbook/Phone.java +++ b/src/main/java/seedu/address/model/profbook/Phone.java @@ -8,7 +8,6 @@ */ public class Phone { - public static final String MESSAGE_CONSTRAINTS = "Phone numbers should only contain numbers, and it should be at least 3 digits long"; public static final Phone PLACEHOLDER = new Phone("n/a"); diff --git a/src/main/java/seedu/address/model/profbook/Root.java b/src/main/java/seedu/address/model/profbook/Root.java index 4f6c6eeb079..b375f3007d6 100644 --- a/src/main/java/seedu/address/model/profbook/Root.java +++ b/src/main/java/seedu/address/model/profbook/Root.java @@ -1,4 +1,5 @@ package seedu.address.model.profbook; + import java.util.Map; import seedu.address.commons.util.ToStringBuilder; @@ -7,7 +8,7 @@ /** * Encapsulates logic for the whole application data */ -public class Root extends ChildrenManager { +public class Root extends ChildManager { /** * Constructs a profbook instance with task list and children. diff --git a/src/main/java/seedu/address/model/profbook/Student.java b/src/main/java/seedu/address/model/profbook/Student.java index c8d56bcc661..c6ee5f7d1fc 100644 --- a/src/main/java/seedu/address/model/profbook/Student.java +++ b/src/main/java/seedu/address/model/profbook/Student.java @@ -51,6 +51,12 @@ public Student(ReadOnlyTaskList taskList, Name name, Email email, Phone phone, A /** * Constructs a new Student without task list. + * + * @param name - The group name + * @param email - The email of the student + * @param phone - Student's Phone number + * @param address - Students address + * @param id - Unique identifier of the group */ public Student(Name name, Email email, Phone phone, Address address, StudentId id) { super(); @@ -63,7 +69,9 @@ public Student(Name name, Email email, Phone phone, Address address, StudentId i } /** - * Construcst a new {@code Student} with the data in {@code toBeCopied}. + * Constructs a new {@code Student} with the data in {@code toBeCopied}. + * + * @param toBeCopied - Data retrieved from storage */ public Student(Student toBeCopied) { super(toBeCopied); @@ -74,22 +82,37 @@ public Student(Student toBeCopied) { this.id = toBeCopied.id; } + /** + * Returns the student's name + */ public Name getName() { return name; } + /** + * Returns the student's id + */ public StudentId getId() { return id; } + /** + * Returns the student's email + */ public Email getEmail() { return email; } + /** + * Returns the student's phone number + */ public Phone getPhone() { return phone; } + /** + * Returns the student's address + */ public Address getAddress() { return address; } @@ -105,7 +128,8 @@ public Student deepCopy() { return new Student(this); } - @Override public int compareTo(Student other) { + @Override + public int compareTo(Student other) { if (this.id.toString().compareTo(other.id.toString()) != 0) { return this.id.toString().compareTo(other.id.toString()); } else { From 852c1bf27075c65993138bc48a38ab378c5f6ae4 Mon Sep 17 00:00:00 2001 From: mingyuanc Date: Wed, 8 Nov 2023 20:43:27 +0800 Subject: [PATCH 4/5] Refactor TaskOperation --- docs/DeveloperGuide.md | 12 +++--- docs/diagrams/ProfBookClassDiagram.puml | 4 +- .../seedu/address/model/ChildOperation.java | 1 + .../seedu/address/model/ITaskOperation.java | 1 + .../seedu/address/model/ModelManager.java | 42 ++++++++++--------- .../seedu/address/model/TaskOperation.java | 1 - .../address/model/profbook/IChildManager.java | 4 +- .../seedu/address/model/profbook/Phone.java | 2 +- .../seedu/address/model/profbook/Root.java | 6 ++- .../seedu/address/model/profbook/Student.java | 3 +- .../address/model/ChildOperationTest.java | 7 ++-- .../address/model/TaskOperationTest.java | 7 ++-- 12 files changed, 49 insertions(+), 41 deletions(-) diff --git a/docs/DeveloperGuide.md b/docs/DeveloperGuide.md index 1c320b4b37a..8195fc7457d 100644 --- a/docs/DeveloperGuide.md +++ b/docs/DeveloperGuide.md @@ -194,8 +194,8 @@ The diagram above shows how the folder structure is implemented in ProfBook, * The hierarchy is as such: `Root` -> `Group` -> `Student` * As many of the operations are repeated (e.g., tasks operations and children operation), we decided to abstract out - these logic into their own classes which is represented by `TaskListManager` and `ChildrenManager` respectively. -* `ChildrenManager` manages the children which is of type `IChildElement` + these logic into their own classes which is represented by `TaskListManager` and `ChildManager` respectively. +* `ChildManager` manages the children which is of type `IChildElement` * We also created a wrapper class (e.g. `ChildrenAndTaskListManager`) for classes that require both of those aforementioned functionalities (e.g, `Group` and potentially in the future `TutorialSlot`) @@ -273,14 +273,14 @@ In our current hierarchy, `Root` -> `Group` -> `Student`, `Student` and `Group` whereas `Root` and `Group` are required to manage children. The `Model` component briefly mentioned this implementation, but I will delve into it more comprehensively. -We first created interfaces to represent the required logic for each of the manager, namely `IChildrenManager` -and `ITaskListManager`. Then we created concrete classes such as `ChildrenManager` and `TaskListManager` to encapsulate +We first created interfaces to represent the required logic for each of the manager, namely `IChildManager` +and `ITaskListManager`. Then we created concrete classes such as `ChildManager` and `TaskListManager` to encapsulate the aforementioned logic. The purpose of these classes was so that should a folder type, e.g. `Student`, require a Manager functionality, we could just extend from said Manager thus reducing on repeated code. Due to the limitation of Java, classes are not able to extend from multiple classes. To remedy this, we created a wrapper class, `ChildrenAndTaskListManager`. -It is important to note that `ChildrenManager` is a generic class that accepts classes that implements +It is important to note that `ChildManager` is a generic class that accepts classes that implements the `IChildElement` interface. This was done to reduce repeated code while introducing a degree of polymorphism. In our implementation, only the parents have reference to the child. This reference is stored by using @@ -849,7 +849,7 @@ in the overall effort. *relative** paths. This component played a crucial role in managing navigation and executing dynamic commands within our application. -- **ChildrenManager Component:** The component was instrumental in representing the hierarchical structure in our +- **ChildManager Component:** The component was instrumental in representing the hierarchical structure in our application. We successfully leveraged this component to perform operations related to child entities, optimizing the handling of students within groups and groups within the ProfBook. diff --git a/docs/diagrams/ProfBookClassDiagram.puml b/docs/diagrams/ProfBookClassDiagram.puml index 024f3bdc190..f969facc7bd 100644 --- a/docs/diagrams/ProfBookClassDiagram.puml +++ b/docs/diagrams/ProfBookClassDiagram.puml @@ -5,10 +5,10 @@ skinparam arrowColor MODEL_COLOR skinparam classBackgroundColor MODEL_COLOR Package ProfBook as ProfBookPackage <>{ -Class "ChildrenManager" as ChildrenManager +Class "ChildManager" as ChildrenManager Class "<>\nIChildElement" as IChildElement Class "<>\nIChildManager" as IChildManager -Class "{abstract}\nChildrenAndTaskListManager" as ChildrenAndTaskListManager +Class "{abstract}\nChildAndTaskListManager" as ChildrenAndTaskListManager Class Root Class Group Class Student diff --git a/src/main/java/seedu/address/model/ChildOperation.java b/src/main/java/seedu/address/model/ChildOperation.java index af70d570f50..184fc5f168c 100644 --- a/src/main/java/seedu/address/model/ChildOperation.java +++ b/src/main/java/seedu/address/model/ChildOperation.java @@ -199,6 +199,7 @@ private List> getAllTaskListManagerChildrenAtLevel(int level) { List> children = new ArrayList<>(getAllChildren()); while (--level > 0) { List> list = new ArrayList<>(); + for (IChildElement child : children) { if (child instanceof ChildAndTaskListManager) { // If child is a group directory diff --git a/src/main/java/seedu/address/model/ITaskOperation.java b/src/main/java/seedu/address/model/ITaskOperation.java index aed6d3b0488..8aa517ca1de 100644 --- a/src/main/java/seedu/address/model/ITaskOperation.java +++ b/src/main/java/seedu/address/model/ITaskOperation.java @@ -10,6 +10,7 @@ * interact with TaskListManager instance */ public interface ITaskOperation { + /** * Adds the tasks to the task list granted it does not result in a duplicate * diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index 9603fec5c96..dbcf48c47d6 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -39,7 +39,6 @@ public class ModelManager implements Model { public static final String MESSAGE_STUDENT_ID_NOT_FOUND = "Student Id must exist in ProfBook."; public static final String MESSAGE_GROUP_NOT_FOUND = "Group must exist in ProfBook."; private static final Logger logger = LogsCenter.getLogger(Model.class); - private final ObservableList displayList = FXCollections.observableArrayList(); private final UserPrefs userPrefs; @@ -59,18 +58,6 @@ public ModelManager() { updateList(); } - /** - * Constructs a model manager with all fields. - */ - public ModelManager(AbsolutePath currPath, Root root, ReadOnlyUserPrefs usePrefs, - AbsolutePath displayPath, boolean showTaskList) { - this(currPath, root, usePrefs); - requireAllNonNull(displayPath, showTaskList); - this.displayPath = displayPath; - this.showTaskList = showTaskList; - updateList(); - } - /** * Construct a model manager with only current path, root (ProfBook) and userPrefs. */ @@ -83,6 +70,18 @@ public ModelManager(AbsolutePath currentPath, Root root, ReadOnlyUserPrefs userP updateList(); } + /** + * Constructs a model manager with all fields. + */ + public ModelManager(AbsolutePath currPath, Root root, ReadOnlyUserPrefs usePrefs, + AbsolutePath displayPath, boolean showTaskList) { + this(currPath, root, usePrefs); + requireAllNonNull(displayPath, showTaskList); + this.displayPath = displayPath; + this.showTaskList = showTaskList; + updateList(); + } + //=========== UserPrefs =================================================================================== @@ -207,11 +206,13 @@ public Student getStudentWithId(StudentId id) { checkArgument(hasStudentWithId(id), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_STUDENT_ID_NOT_FOUND)); logger.info("Finding student with id: " + id); - for (Group group : this.root.getAllChildren()) { + + for (Group group : this.root.getAllChildren()) { // for each group, check if group has student if (group.hasChild(id)) { - return group.getChild(id); + return group.getChild(id); // if student is present, return student } } + // If student is not present, throw error as user should have check if present logger.severe("Unable to find student with id: " + id); throw new IllegalArgumentException(String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_UNEXPECTED_ERROR)); } @@ -361,6 +362,7 @@ public ChildOperation groupChildOperation(AbsolutePath path) { String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_GROUP_INFO_NOT_FOUND)); checkArgument(hasGroup(path), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_GROUP_NOT_FOUND)); + logger.info("New GroupChildOperation at group: " + path); return new ChildOperation<>(getGroupFromPath(path)); } @@ -373,6 +375,7 @@ public TaskOperation taskOperation(AbsolutePath path) { checkArgument(hasPath(path), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_PATH_NOT_FOUND)); logger.info("New TaskOperation"); + if (path.isGroupDirectory()) { return new TaskOperation(getGroupFromPath(path)); } @@ -395,8 +398,8 @@ public boolean hasChildrenListInPath(AbsolutePath path) { /** * Return the group of the given path. - * {@code path} must has a valid group info - * i.e group exists in ProfBook. + * + * @param path - must point to a valid and present group */ private Group getGroupFromPath(AbsolutePath path) { requireNonNull(path); @@ -418,7 +421,8 @@ private Group getGroupFromPath(AbsolutePath path) { /** * Return the group of the given path. - * {@code path} must be student path that exists in ProfBook. + * + * @param path - must point to a valid and present group */ private Student getStudentFromPath(AbsolutePath path) { requireNonNull(path); @@ -440,7 +444,7 @@ private Student getStudentFromPath(AbsolutePath path) { } /** - * Return the Root of addressbook. + * Return the Root of ProfBook. */ @Override public Root getRoot() { diff --git a/src/main/java/seedu/address/model/TaskOperation.java b/src/main/java/seedu/address/model/TaskOperation.java index 65e60cecbbc..fc067119c62 100644 --- a/src/main/java/seedu/address/model/TaskOperation.java +++ b/src/main/java/seedu/address/model/TaskOperation.java @@ -20,7 +20,6 @@ public class TaskOperation implements ITaskOperation { private static final String MESSAGE_DUPLICATE_TASK = "Task must not exist in task list."; private static final String MESSAGE_TASK_NOT_FOUND = "Task not found in task list."; private final Logger logger = LogsCenter.getLogger(JsonUtil.class); - private final ITaskListManager baseDir; /** diff --git a/src/main/java/seedu/address/model/profbook/IChildManager.java b/src/main/java/seedu/address/model/profbook/IChildManager.java index 6e023d86681..a1eb72750b6 100644 --- a/src/main/java/seedu/address/model/profbook/IChildManager.java +++ b/src/main/java/seedu/address/model/profbook/IChildManager.java @@ -8,7 +8,9 @@ import seedu.address.model.profbook.exceptions.NoSuchChildException; /** - * API for Children Manager + * Encapsulates the required logic for IChildManager + * + * @param type of child being stored */ public interface IChildManager> { diff --git a/src/main/java/seedu/address/model/profbook/Phone.java b/src/main/java/seedu/address/model/profbook/Phone.java index 2c2621866ff..afde28d8782 100644 --- a/src/main/java/seedu/address/model/profbook/Phone.java +++ b/src/main/java/seedu/address/model/profbook/Phone.java @@ -15,7 +15,7 @@ public class Phone { public final String value; /** - * Constructs a {@code Phone}. + * Constructs a Phone instance * * @param phone A valid phone number. */ diff --git a/src/main/java/seedu/address/model/profbook/Root.java b/src/main/java/seedu/address/model/profbook/Root.java index b375f3007d6..167eecead1f 100644 --- a/src/main/java/seedu/address/model/profbook/Root.java +++ b/src/main/java/seedu/address/model/profbook/Root.java @@ -11,7 +11,7 @@ public class Root extends ChildManager { /** - * Constructs a profbook instance with task list and children. + * Constructs a prof book instance with task list and children. * * @param children - The Groups under the root */ @@ -27,7 +27,9 @@ public Root() { } /** - * Constructs a {@code Root} with the data in {@code toBeCopied} + * Constructs a new Root instance with the data in toBeCopied + * + * @param toBeCopied - Data extracted from storage */ public Root(Root toBeCopied) { super(toBeCopied); diff --git a/src/main/java/seedu/address/model/profbook/Student.java b/src/main/java/seedu/address/model/profbook/Student.java index c6ee5f7d1fc..13360d7dd6e 100644 --- a/src/main/java/seedu/address/model/profbook/Student.java +++ b/src/main/java/seedu/address/model/profbook/Student.java @@ -69,7 +69,7 @@ public Student(Name name, Email email, Phone phone, Address address, StudentId i } /** - * Constructs a new {@code Student} with the data in {@code toBeCopied}. + * Constructs a new instance with the data in toBeCopied. * * @param toBeCopied - Data retrieved from storage */ @@ -122,7 +122,6 @@ public StudentCard getDisplayCard(int displayedIndex) { return new StudentCard(this, displayedIndex); } - @Override public Student deepCopy() { return new Student(this); diff --git a/src/test/java/seedu/address/model/ChildOperationTest.java b/src/test/java/seedu/address/model/ChildOperationTest.java index 913789bc673..7d9bd07b3fb 100644 --- a/src/test/java/seedu/address/model/ChildOperationTest.java +++ b/src/test/java/seedu/address/model/ChildOperationTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -147,14 +148,12 @@ public void hashCode_equalChildOperations_returnsSameHashCode() { public void equals_sameChildOperations_returnsTrue() { ChildOperation opr = model.groupChildOperation(grpPath); ChildOperation opr2 = model.groupChildOperation(grpPath); - assertTrue(opr.equals(opr2)); + assertEquals(opr, opr2); } @Test public void equals_nullChildOperations_returnsTrue() { ChildOperation opr = model.groupChildOperation(grpPath); - assertFalse(opr.equals(null)); + assertNotNull(opr); } - - } diff --git a/src/test/java/seedu/address/model/TaskOperationTest.java b/src/test/java/seedu/address/model/TaskOperationTest.java index e1f5216f164..2d3a211a42b 100644 --- a/src/test/java/seedu/address/model/TaskOperationTest.java +++ b/src/test/java/seedu/address/model/TaskOperationTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -58,6 +59,7 @@ public void init() { .withPhone("98765432") .withAddress("311, Clementi Ave 2, #02-25") .withId("0001Y").build(); + Map studentMap = new HashMap<>(); studentMap.put(new StudentId("0001Y"), this.student); this.group = new Group(new TaskListManager(), studentMap, new Name("gary"), new GroupId("grp-001")); @@ -90,7 +92,6 @@ public void taskOperationVerifyDeleteMethod_noErrorReturn() { } catch (NoSuchTaskException e) { fail(); } - //assertFalse(this.student.contains(task)); } @Test @@ -140,13 +141,13 @@ public void taskOperation_checkHashCode() { public void equals_sameTaskOperations_returnsTrue() { TaskOperation opr1 = model.taskOperation(stuPath); TaskOperation opr2 = model.taskOperation(stuPath); - assertTrue(opr1.equals(opr2)); + assertEquals(opr1, opr2); } @Test public void equals_nullTaskOperations_returnsFalse() { TaskOperation opr1 = model.taskOperation(stuPath); - assertFalse(opr1.equals(null)); + assertNotEquals(null, opr1); } @Test From b4b9fd9484844d73e8cd5e18dcfc7b7db42e4060 Mon Sep 17 00:00:00 2001 From: Ming Yuan Date: Sat, 11 Nov 2023 13:46:23 +0800 Subject: [PATCH 5/5] Refactor code to sound boolean --- .../logic/commands/CreateDeadlineCommand.java | 27 +++++++------------ .../logic/commands/CreateTodoCommand.java | 23 +++++----------- .../seedu/address/model/ChildOperation.java | 4 +-- .../seedu/address/model/IChildOperation.java | 4 +-- 4 files changed, 20 insertions(+), 38 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/CreateDeadlineCommand.java b/src/main/java/seedu/address/logic/commands/CreateDeadlineCommand.java index 4c2d2853227..36b577ad2c8 100644 --- a/src/main/java/seedu/address/logic/commands/CreateDeadlineCommand.java +++ b/src/main/java/seedu/address/logic/commands/CreateDeadlineCommand.java @@ -30,8 +30,8 @@ public class CreateDeadlineCommand extends Command { + " -d, --desc Description of the deadline task\n" + " -dt, --datetime Deadline of the task\n" + " Format: yyyy-MM-dd HH:mm\n" - + " \'yyyy\': year, \'MM\': month, 'dd\': day,\n" - + " \'HH\': hour (24-hour format), 'mm\': minutes.\n" + + " 'yyyy': year, 'MM': month, 'dd': day,\n" + + " 'HH': hour (24-hour format), 'mm': minutes.\n" + "\n" + "Option: \n" + " path Valid path to group or student\n" @@ -78,7 +78,7 @@ public class CreateDeadlineCommand extends Command { public static final CreateDeadlineCommand HELP_MESSAGE = new CreateDeadlineCommand() { @Override - public CommandResult execute(Model model) throws CommandException { + public CommandResult execute(Model model) { return new CommandResult(MESSAGE_USAGE); } }; @@ -190,15 +190,12 @@ private CommandResult addTaskToAllStuInGrp(Model model) throws CommandException ChildOperation groupOper = model.groupChildOperation(path); // Check whether all children already have the task - if (groupOper.checkIfAllChildrenHaveTask(deadline, 1)) { + if (groupOper.doAllChildrenHaveTasks(deadline, 1)) { throw new CommandException(String.format(MESSAGE_ALL_CHILDREN_HAVE_TASK, "student")); } // Check whether at least one of the children has the task - boolean warning = false; - if (groupOper.checkIfAnyChildHasTask(deadline, 1)) { - warning = true; - } + boolean warning = groupOper.doAnyChildrenHaveTasks(deadline, 1); groupOper.addTaskToAllChildren(deadline, 1); model.updateList(); @@ -217,15 +214,12 @@ private CommandResult addTaskToAllStuInRoot(Model model) throws CommandException ChildOperation operation = model.rootChildOperation(); // Check whether all children already have the task - if (operation.checkIfAllChildrenHaveTask(deadline, 2)) { + if (operation.doAllChildrenHaveTasks(deadline, 2)) { throw new CommandException(String.format(MESSAGE_ALL_CHILDREN_HAVE_TASK, "student")); } // Check whether at least one of the children has the task - boolean warning = false; - if (operation.checkIfAnyChildHasTask(deadline, 2)) { - warning = true; - } + boolean warning = operation.doAnyChildrenHaveTasks(deadline, 2); operation.addTaskToAllChildren(deadline, 2); model.updateList(); @@ -258,15 +252,12 @@ private CommandResult addTaskToAllGrpInRoot(Model model) throws CommandException ChildOperation rootOper = model.rootChildOperation(); // Check whether all children already have the task - if (rootOper.checkIfAllChildrenHaveTask(deadline, 1)) { + if (rootOper.doAllChildrenHaveTasks(deadline, 1)) { throw new CommandException(String.format(MESSAGE_ALL_CHILDREN_HAVE_TASK, "group")); } // Check whether at least one of the children has the task - boolean warning = false; - if (rootOper.checkIfAnyChildHasTask(deadline, 1)) { - warning = true; - } + boolean warning = rootOper.doAnyChildrenHaveTasks(deadline, 1); rootOper.addTaskToAllChildren(deadline, 1); model.updateList(); diff --git a/src/main/java/seedu/address/logic/commands/CreateTodoCommand.java b/src/main/java/seedu/address/logic/commands/CreateTodoCommand.java index ac979f12214..54dfb5a327a 100644 --- a/src/main/java/seedu/address/logic/commands/CreateTodoCommand.java +++ b/src/main/java/seedu/address/logic/commands/CreateTodoCommand.java @@ -74,7 +74,7 @@ public class CreateTodoCommand extends Command { public static final CreateTodoCommand HELP_MESSAGE = new CreateTodoCommand() { @Override - public CommandResult execute(Model model) throws CommandException { + public CommandResult execute(Model model) { return new CommandResult(MESSAGE_USAGE); } }; @@ -185,15 +185,12 @@ private CommandResult addTaskToAllStuInGrp(Model model) throws CommandException ChildOperation groupOper = model.groupChildOperation(path); // Check whether all children already have the task - if (groupOper.checkIfAllChildrenHaveTask(todo, 1)) { + if (groupOper.doAllChildrenHaveTasks(todo, 1)) { throw new CommandException(String.format(MESSAGE_ALL_CHILDREN_HAVE_TASK, "student")); } // Check whether at least one of the children has the task - boolean warning = false; - if (groupOper.checkIfAnyChildHasTask(todo, 1)) { - warning = true; - } + boolean warning = groupOper.doAnyChildrenHaveTasks(todo, 1); groupOper.addTaskToAllChildren(todo, 1); model.updateList(); @@ -212,15 +209,12 @@ private CommandResult addTaskToAllStuInRoot(Model model) throws CommandException ChildOperation operation = model.rootChildOperation(); // Check whether all children already have the task - if (operation.checkIfAllChildrenHaveTask(todo, 2)) { + if (operation.doAllChildrenHaveTasks(todo, 2)) { throw new CommandException(String.format(MESSAGE_ALL_CHILDREN_HAVE_TASK, "student")); } // Check whether at least one of the children has the task - boolean warning = false; - if (operation.checkIfAnyChildHasTask(todo, 2)) { - warning = true; - } + boolean warning = operation.doAnyChildrenHaveTasks(todo, 2); operation.addTaskToAllChildren(todo, 2); model.updateList(); @@ -253,15 +247,12 @@ private CommandResult addTaskToAllGrpInRoot(Model model) throws CommandException ChildOperation rootOper = model.rootChildOperation(); // Check whether all children already have the task - if (rootOper.checkIfAllChildrenHaveTask(todo, 1)) { + if (rootOper.doAllChildrenHaveTasks(todo, 1)) { throw new CommandException(String.format(MESSAGE_ALL_CHILDREN_HAVE_TASK, "group")); } // Check whether at least one of the children has the task - boolean warning = false; - if (rootOper.checkIfAnyChildHasTask(todo, 1)) { - warning = true; - } + boolean warning = rootOper.doAnyChildrenHaveTasks(todo, 1); rootOper.addTaskToAllChildren(todo, 1); model.updateList(); diff --git a/src/main/java/seedu/address/model/ChildOperation.java b/src/main/java/seedu/address/model/ChildOperation.java index 3e8e17b780a..ca999c20170 100644 --- a/src/main/java/seedu/address/model/ChildOperation.java +++ b/src/main/java/seedu/address/model/ChildOperation.java @@ -117,7 +117,7 @@ public int numOfChildren() { } @Override - public boolean checkIfAllChildrenHaveTask(Task task, int level) { + public boolean doAllChildrenHaveTasks(Task task, int level) { List> children = getAllTaskListManagerChildrenAtLevel(level); for (IChildElement child : children) { @@ -137,7 +137,7 @@ public boolean checkIfAllChildrenHaveTask(Task task, int level) { } @Override - public boolean checkIfAnyChildHasTask(Task task, int level) { + public boolean doAnyChildrenHaveTasks(Task task, int level) { List> children = getAllTaskListManagerChildrenAtLevel(level); for (IChildElement child : children) { diff --git a/src/main/java/seedu/address/model/IChildOperation.java b/src/main/java/seedu/address/model/IChildOperation.java index c65fae820a7..be6a7a7c698 100644 --- a/src/main/java/seedu/address/model/IChildOperation.java +++ b/src/main/java/seedu/address/model/IChildOperation.java @@ -78,14 +78,14 @@ public interface IChildOperation> { * The {@code level} must be level of task list manager. * e.g. Group level and student level. */ - boolean checkIfAllChildrenHaveTask(Task task, int level); + boolean doAllChildrenHaveTasks(Task task, int level); /** * Returns {@code true} if at least one child at {@code level} has the task. * The {@code level} must be level of task list manager. * e.g. Group level and student level. */ - boolean checkIfAnyChildHasTask(Task task, int level); + boolean doAnyChildrenHaveTasks(Task task, int level); /** * Returns Number of current children