diff --git a/docs/DeveloperGuide.md b/docs/DeveloperGuide.md index 815358647a8..369d793efee 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`) @@ -291,14 +291,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 @@ -1621,7 +1621,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/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 809da87e099..ca999c20170 100644 --- a/src/main/java/seedu/address/model/ChildOperation.java +++ b/src/main/java/seedu/address/model/ChildOperation.java @@ -8,9 +8,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; @@ -23,14 +23,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; } @@ -117,57 +117,68 @@ public int numOfChildren() { } @Override - public void addTaskToAllChildren(Task task, int level) { + public boolean doAllChildrenHaveTasks(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 doAnyChildrenHaveTasks(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,12 +200,15 @@ private List> getAllTaskListManagerChildrenAtLevel(int level) { List> children = new ArrayList<>(getAllChildren()); 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 add804943f9..be6a7a7c698 100644 --- a/src/main/java/seedu/address/model/IChildOperation.java +++ b/src/main/java/seedu/address/model/IChildOperation.java @@ -17,6 +17,7 @@ * @param Type of the child */ public interface IChildOperation> { + /** * Adds the child to list of children * @@ -77,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 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 12481b90070..4368b836922 100644 --- a/src/main/java/seedu/address/model/ITaskOperations.java +++ b/src/main/java/seedu/address/model/ITaskOperation.java @@ -10,14 +10,14 @@ * 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 { +public interface ITaskOperation { /** - * Checks if current task is present + * Adds the tasks to the task list granted it does not result in a duplicate * * @param t The task in question */ - boolean hasTask(Task t); + void addTask(Task t); /** * Check if index is between 0 and task list size. @@ -25,17 +25,16 @@ public interface ITaskOperations { boolean isValidIndex(int index); /** - * Return the size of the task list. + * Checks if current task is present + * + * @param t The task in question */ - int getTaskListSize(); + boolean hasTask(Task t); /** - * Adds a new tasks to the task list. - * Task must not be duplicated class. - * - * @param t The task in question + * Return the current size of the task list. */ - void addTask(Task t); + 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 0cf7af26082..f053ae37c31 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -28,35 +28,46 @@ * 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() { + this.currentPath = AbsolutePath.ROOT_PATH; + this.displayPath = AbsolutePath.ROOT_PATH; + this.root = new Root(); + this.userPrefs = new UserPrefs(); + updateList(); + } + + /** + * Construct a model manager with only current path, root (ProfBook) and userPrefs. */ public ModelManager(AbsolutePath currentPath, Root root, ReadOnlyUserPrefs userPrefs) { requireAllNonNull(currentPath, root, userPrefs); - this.currentPath = currentPath; + this.userPrefs = new UserPrefs(userPrefs); this.displayPath = currentPath; + this.currentPath = currentPath; this.root = new Root(root); - this.userPrefs = new UserPrefs(userPrefs); updateList(); } @@ -64,7 +75,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; @@ -72,49 +83,70 @@ public ModelManager(AbsolutePath currPath, Root root, ReadOnlyUserPrefs usePrefs updateList(); } - /** - * Constructs a new model manager with empty data. - */ - public ModelManager() { - this.currentPath = AbsolutePath.ROOT_PATH; - this.displayPath = AbsolutePath.ROOT_PATH; - this.root = new Root(); - this.userPrefs = new UserPrefs(); - 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; @@ -123,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 @@ -138,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 @@ -152,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), @@ -173,19 +206,32 @@ public Group getGroupWithId(GroupId id) { public Student getStudentWithId(StudentId id) { checkArgument(hasStudentWithId(id), String.format(MESSAGE_INTERNAL_ERROR, MESSAGE_STUDENT_ID_NOT_FOUND)); - for (Group group : this.root.getAllChildren()) { + logger.info("Finding student with id: " + id); + + 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)); } + @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); } @@ -195,20 +241,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; } @@ -227,8 +281,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(); } @@ -241,19 +295,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 @@ -261,39 +317,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 @@ -303,6 +363,8 @@ 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)); } @@ -313,6 +375,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)); @@ -336,8 +399,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); @@ -346,6 +409,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); @@ -353,7 +422,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); @@ -362,6 +432,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(); @@ -369,7 +445,7 @@ private Student getStudentFromPath(AbsolutePath path) { } /** - * Return the Root of addressbook. + * Return the Root of ProfBook. */ @Override public Root getRoot() { @@ -406,5 +482,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 ee5464afc7b..cba6705e964 100644 --- a/src/main/java/seedu/address/model/TaskOperation.java +++ b/src/main/java/seedu/address/model/TaskOperation.java @@ -11,50 +11,77 @@ 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 { +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 ITaskListManager baseDir; private final Logger logger = LogsCenter.getLogger(JsonUtil.class); + private final ITaskListManager baseDir; + /** + * Constructs a TaskOperation instance + * + * @param baseDir - The current directory to enact task operations + */ public TaskOperation(ITaskListManager baseDir) { this.baseDir = baseDir; } - void stateLogger(String log) { + private void stateLogger(String log) { this.logger.info(log); } - void stateErrorLogger(String errMsg) { - this.logger.severe(errMsg); - } - + /** + * Checks if current task is present + * + * @param task The task in question + */ @Override - public int getTaskListSize() { - return this.baseDir.size(); + 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 task 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); @@ -62,6 +89,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); @@ -69,6 +103,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); @@ -76,12 +117,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); @@ -89,6 +143,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 "); 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 90% rename from src/main/java/seedu/address/model/profbook/IChildrenManager.java rename to src/main/java/seedu/address/model/profbook/IChildManager.java index 1a798da1108..a1eb72750b6 100644 --- a/src/main/java/seedu/address/model/profbook/IChildrenManager.java +++ b/src/main/java/seedu/address/model/profbook/IChildManager.java @@ -8,9 +8,12 @@ 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 IChildrenManager> { +public interface IChildManager> { + /** * Adds the child to list of children * @@ -58,7 +61,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..afde28d8782 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"); @@ -16,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 4f6c6eeb079..167eecead1f 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,10 +8,10 @@ /** * 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. + * Constructs a prof book instance with task list and children. * * @param children - The Groups under the root */ @@ -26,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 c8d56bcc661..13360d7dd6e 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 instance with the data in 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; } @@ -99,13 +122,13 @@ public StudentCard getDisplayCard(int displayedIndex) { return new StudentCard(this, displayedIndex); } - @Override 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 { 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