Merge pull request #257 from AY2122S1-CS2103-W14-4/branch-optimisation

docs/diagrams/LogicClassDiagram.puml

@@ -9,7 +9,7 @@ package Logic {
 Class AddressBookParser
 Class XYZCommand
 Class CommandResult
-Class "{abstract}\nCommand" as Command
+abstract Class "{abstract}\nCommand" as Command
 
 
 Interface Logic <<Interface>>

docs/diagrams/TaskClassDiagram.puml

@@ -5,7 +5,7 @@ skinparam arrowColor MODEL_COLOR
 skinparam classBackgroundColor MODEL_COLOR
 
 Package Task <<Rectangle>> {
-abstract Class Task
+abstract Class "{abstract}\nTask" as Task
 Class TaskName
 Class Description
 Class TaskDate

docs/team/itsraveen.md

@@ -5,6 +5,8 @@ title: Raveen's Project Portfolio Page
 
 ### Project: tApp
 
+tApp is a desktop app for managing tutorial groups and personal tasks, optimized for use via a Command Line Interface (CLI). It has a GUI created with JavaFX. It is written in Java, and has about 15 kLoC.
+
 Given below are my contributions to the project.
 
 * **Code contributed:** [RepoSense link](https://nus-cs2103-ay2122s1.github.io/tp-dashboard/?search=raveen)

src/main/java/seedu/address/logic/commands/MarkTaskDoneCommand.java

@@ -39,6 +39,12 @@ public CommandResult execute(Model model) throws CommandException {
         List<Task> savedStateList = new ArrayList<>(model.getFilteredTaskList());
         StringBuilder result = new StringBuilder();
 
+        for (Index targetIndex : targetIndexList) {
+            if (targetIndex.getZeroBased() >= savedStateList.size()) {
+                throw new CommandException(Messages.MESSAGE_INVALID_TASK_DISPLAYED_INDEX);
+            }
+        }
+
         for (Index targetIndex : targetIndexList) {
             if (targetIndex.getZeroBased() >= savedStateList.size()) {
                 throw new CommandException(Messages.MESSAGE_INVALID_TASK_DISPLAYED_INDEX);

src/main/java/seedu/address/model/AddressBook.java

@@ -246,7 +246,6 @@ public void addGroup(Group g) {
      */
     public void setGroup(Group target, Group editedGroup) {
         requireNonNull(editedGroup);
-
         groups.setGroup(target, editedGroup);
     }
 

src/main/java/seedu/address/model/ModelManager.java

@@ -171,7 +171,7 @@ public void markStudentAttendance(Student target, int week) {
         requireAllNonNull(target, week);
         Student newStudent = target.clone();
         newStudent.toggleAttendance(week);
-        updateGroup(target, newStudent);
+        setStudent(target, newStudent);
     }
 
     @Override
@@ -185,7 +185,7 @@ public void markStudentParticipation(Student target, int week) {
         requireAllNonNull(target, week);
         Student newStudent = target.clone();
         newStudent.toggleParticipation(week);
-        updateGroup(target, newStudent);
+        setStudent(target, newStudent);
     }
 
     @Override
@@ -198,8 +198,10 @@ public String getStudentParticipation(Student target, int week) {
     public void addMember(Student student, Group group) {
         requireAllNonNull(student, group);
         Student updatedStudent = new Student(student, group.getName());
-        group.addMember(updatedStudent);
+        Group newGroup = group.clone();
+        newGroup.addMember(updatedStudent);
         addressBook.setStudent(student, updatedStudent);
+        addressBook.setGroup(group, newGroup);
     }
 
     @Override
@@ -257,6 +259,7 @@ public void deleteGroup(Group target) {
         List<Student> students = target.getMembersList();
         addressBook.clearGroupFromStudents(students);
         addressBook.deleteGroup(target);
+        displayType = GROUPS;
     }
 
     @Override

src/main/java/seedu/address/model/group/Group.java

@@ -35,35 +35,17 @@ public class Group {
     public Group(GroupName name, Members members, LinkYear year, RepoName repoName, Set<Tag> tags) {
         requireNonNull(name);
         this.name = name;
-        if (members != null) {
-            this.members = members;
-        } else {
-            this.members = new Members();
-        }
-        if (year != null) {
-            this.year = year;
-        } else {
-            this.year = new LinkYear();
-        }
-        if (repoName != null) {
-            this.repoName = repoName;
-        } else {
-            this.repoName = new RepoName();
-        }
-
+        this.members = Objects.requireNonNullElseGet(members, Members::new);
+        this.year = Objects.requireNonNullElseGet(year, LinkYear::new);
+        this.repoName = Objects.requireNonNullElseGet(repoName, RepoName::new);
         this.tags.addAll(tags);
     }
 
     /**
      * Constructor for a new Group object given only name and tags
      */
     public Group(GroupName name, Set<Tag> tags) {
-        requireAllNonNull(name);
-        this.name = name;
-        this.members = new Members();
-        this.tags.addAll(tags);
-        this.year = new LinkYear();
-        this.repoName = new RepoName();
+        this(name, new Members(), new LinkYear(), new RepoName(), tags);
     }
 
     public GroupName getName() {
@@ -113,7 +95,7 @@ public void removeAllMembers() {
     }
 
     /**
-     * Returns the formatted Github link
+     * Returns the formatted GitHub link
      */
     public String getGroupGithubLink() {
         if (!year.isNull() && !repoName.isNull()) {
@@ -123,20 +105,6 @@ public String getGroupGithubLink() {
         }
     }
 
-    /**
-     * Returns the formatted Github link with given inputs
-     * @param year A valid year to parse
-     * @param repoName A valid repoName to parse
-     */
-    public String getGroupGithubLink(LinkYear year, RepoName repoName) {
-        if (!year.isNull() && !repoName.isNull()) {
-            return String.format(new GroupGithub(year, repoName).toString(), getName());
-        } else {
-            return "-";
-        }
-    }
-
-
     /**
      * Returns an immutable tag set, which throws {@code UnsupportedOperationException}
      * if modification is attempted.
@@ -192,7 +160,7 @@ public String toString() {
                 .append("; Members: ")
                 .append(getMembers())
                 .append("; Github: ")
-                .append(getGroupGithubLink());;
+                .append(getGroupGithubLink());
 
         Set<Tag> tags = getTags();
         if (!tags.isEmpty()) {
@@ -203,6 +171,15 @@ public String toString() {
         return builder.toString();
     }
 
+    /**
+     * Makes a shallow copy of a group.
+     *
+     * @return a cloned Group with the exact same data fields as the original.
+     */
+    public Group clone() {
+        return new Group(name, new Members(getMembersList()), year, repoName, new HashSet<>(tags));
+    }
+
     /**
      * Represents a Group's name in tApp.
      * Guarantees: immutable;

src/main/java/seedu/address/model/group/GroupName.java

@@ -37,7 +37,7 @@ public GroupName() {
     }
 
     public boolean isNull () {
-        return name == null ? true : false;
+        return name == null;
     }
 
     /**

src/main/java/seedu/address/model/group/Members.java

@@ -1,25 +1,24 @@
 package seedu.address.model.group;
 
-import java.util.ArrayList;
+import java.util.List;
 
+import javafx.collections.FXCollections;
+import javafx.collections.ObservableList;
 import seedu.address.model.student.Student;
 
 public class Members {
 
-    public final ArrayList<Student> studentList;
+    public final ObservableList<Student> studentList = FXCollections.observableArrayList();
 
     /**
      * Constructs a {@code Members}.
      */
-    public Members() {
-        studentList = new ArrayList<>();
-    }
+    public Members() { }
 
     /**
      * Constructs a existing {@code Members}.
      */
-    public Members(ArrayList<Student> studentList) {
-        this.studentList = new ArrayList<>();
+    public Members(List<Student> studentList) {
         this.studentList.addAll(studentList);
     }
 

src/main/java/seedu/address/model/group/UniqueGroupList.java

@@ -122,7 +122,7 @@ public void removeStudentFromGroup(Student student, Group group) {
             throw new GroupNotFoundException();
         }
 
-        Group updatedGroup = group;
+        Group updatedGroup = group.clone();
         updatedGroup.removeMember(student);
 
         if (!group.isSameGroup(updatedGroup) && contains(updatedGroup)) {

src/main/java/seedu/address/model/student/Student.java

@@ -36,24 +36,8 @@ public class Student {
      */
     public Student(Name name, Email email, StudentNumber studentNumber, UserName userName, RepoName repoName,
                    Set<Tag> tags) {
-        requireAllNonNull(name, email, studentNumber, tags);
-        this.name = name;
-        this.email = email;
-        this.studentNumber = studentNumber;
-        this.tags.addAll(tags);
-        this.attendance = new Attendance();
-        this.participation = new Participation();
-        if (userName != null) {
-            this.userName = userName;
-        } else {
-            this.userName = new UserName();
-        }
-        if (repoName != null) {
-            this.repoName = repoName;
-        } else {
-            this.repoName = new RepoName();
-        }
-        this.groupName = new GroupName();
+        this(name, email, studentNumber, userName, repoName, tags,
+                new Attendance(), new Participation(), new GroupName());
     }
 
     /**
@@ -62,23 +46,15 @@ public Student(Name name, Email email, StudentNumber studentNumber, UserName use
 
     public Student(Name name, Email email, StudentNumber studentNumber, UserName userName, RepoName repoName,
                    Set<Tag> tags, Attendance attendance, Participation participation, GroupName groupName) {
-        requireAllNonNull(name, email, studentNumber, tags, attendance);
+        requireAllNonNull(name, email, studentNumber, tags, attendance, participation);
         this.name = name;
         this.email = email;
         this.studentNumber = studentNumber;
         this.tags.addAll(tags);
         this.attendance = attendance;
         this.participation = participation;
-        if (userName != null) {
-            this.userName = userName;
-        } else {
-            this.userName = new UserName();
-        }
-        if (repoName != null) {
-            this.repoName = repoName;
-        } else {
-            this.repoName = new RepoName();
-        }
+        this.userName = Objects.requireNonNullElseGet(userName, UserName::new);
+        this.repoName = Objects.requireNonNullElseGet(repoName, RepoName::new);
         this.groupName = groupName;
     }
 
@@ -87,16 +63,8 @@ public Student(Name name, Email email, StudentNumber studentNumber, UserName use
      */
 
     public Student(Student student, GroupName groupName) {
-        requireAllNonNull(student, groupName);
-        this.name = student.getName();
-        this.email = student.getEmail();
-        this.studentNumber = student.getStudentNumber();
-        this.userName = student.getUserName();
-        this.repoName = student.getRepoName();
-        this.tags.addAll(student.getTags());
-        this.attendance = student.getAttendance();
-        this.participation = student.getParticipation();
-        this.groupName = groupName;
+        this(student.name, student.email, student.studentNumber, student.userName, student.repoName, student.tags,
+                student.getAttendance(), student.getParticipation(), groupName);
     }
 
     public Name getName() {
@@ -187,9 +155,7 @@ public boolean hasGroupName() {
      * @return a cloned Student with the exact same data fields as the original.
      */
     public Student clone() {
-        Student clone = new Student(name, email, studentNumber, userName, repoName, tags, attendance,
-                participation, groupName);
-        return clone;
+        return new Student(name, email, studentNumber, userName, repoName, tags, attendance, participation, groupName);
     }
 
     /**
@@ -243,7 +209,7 @@ public String toString() {
     }
 
     /**
-     * Represents a Student's github ip link in tApp.
+     * Represents a Student's GitHub ip link in tApp.
      * Guarantees: immutable;
      */
     public static class GithubLink {

src/main/java/seedu/address/model/tag/Tag.java

@@ -9,8 +9,9 @@
  */
 public class Tag {
 
-    public static final String MESSAGE_CONSTRAINTS = "Tags names should be alphanumeric with no spaces.";
-    public static final String VALIDATION_REGEX = "\\p{Alnum}+";
+    public static final String MESSAGE_CONSTRAINTS =
+            "Tags names should be alphanumeric, contain no spaces, and must be between 1 to 20 characters long.";
+    public static final String VALIDATION_REGEX = "^[a-zA-Z0-9]{1,20}$";
 
     public final String tagName;
 

src/main/java/seedu/address/model/task/Task.java

@@ -1,5 +1,7 @@
 package seedu.address.model.task;
 
+import static seedu.address.commons.util.CollectionUtil.requireAllNonNull;
+
 import java.time.LocalDate;
 import java.util.Collections;
 import java.util.HashSet;
@@ -34,6 +36,7 @@ public enum Priority {
      * @param priority A valid priority for the Task.
      */
     public Task(TaskName name, Set<Tag> tags, boolean isDone, Description description, Priority priority) {
+        requireAllNonNull(name, priority);
         this.name = name;
         this.tags.addAll(tags);
         this.isDone = isDone;
@@ -50,11 +53,7 @@ public Task(TaskName name, Set<Tag> tags, boolean isDone, Description descriptio
      * @param priority A valid Priority.
      */
     public Task(TaskName name, Set<Tag> tags, Description description, Priority priority) {
-        this.name = name;
-        this.tags.addAll(tags);
-        this.isDone = false;
-        this.description = description;
-        this.priority = priority;
+        this(name, tags, false, description, priority);
     }
 
     public TaskName getName() {

src/main/java/seedu/address/ui/GroupCard.java

@@ -2,7 +2,6 @@
 
 import java.util.Comparator;
 
-import javafx.collections.FXCollections;
 import javafx.fxml.FXML;
 import javafx.scene.control.Label;
 import javafx.scene.layout.FlowPane;
@@ -12,7 +11,7 @@
 import seedu.address.model.group.Group;
 
 /**
- * An UI component that displays information of a {@code Group}.
+ * A UI component that displays information of a {@code Group}.
  */
 public class GroupCard extends UiPart<Region> {
 
@@ -51,8 +50,7 @@ public GroupCard(Group group, int displayedIndex) {
         id.setText(displayedIndex + ". ");
         name.setText(group.getName().name);
         link.setText("Github: " + group.getGroupGithubLink());
-        memberListPanel = new MemberListPanel(
-                FXCollections.observableArrayList(group.getMembers().studentList));
+        memberListPanel = new MemberListPanel(group.getMembers().studentList);
 
         memberListPanelPlaceholder.getChildren().add(memberListPanel.getRoot());