v1.2.1 attendence #81

YuunoKun · 2020-10-02T14:44:02Z

Implements new Attendance feature together with Session/SessionList.
This PR does not guarantee the functionality of the Participate and Presence command, as it can only be completely done during integration (It needs to 'enter a session' which will have to acquire SessionName internally).

# Conflicts: # src/main/java/seedu/address/commons/core/Messages.java # src/main/java/seedu/address/logic/parser/AddressBookParser.java

marcustw

Excellent work Sheng Yang!

marcustw · 2020-10-02T15:47:51Z

src/main/java/seedu/address/commons/core/Messages.java

@@ -9,6 +9,7 @@
    public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s";
    public static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The student index provided is invalid";
    public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d students listed!";
+    public static final String MESSAGE_SESSION_DOES_NOT_EXIST = "The session does not exist";


Perhaps missing a fullstop?

Not sure about this, the punctuation isn't consistent for other message strings too. I guess we can clean this up after integration.

Sure thing!

src/main/java/seedu/address/model/attendance/SessionDate.java

marcustw · 2020-10-02T15:59:34Z

src/main/java/seedu/address/model/attendance/SessionDate.java

+    }
+
+    /**
+     * Standardize date string into the format of dd/MM/yyyy.


Perhaps "Standardizes" instead of "Standardize"?

marcustw · 2020-10-02T16:01:12Z

src/main/java/seedu/address/model/attendance/exceptions/SessionNotFoundException.java

+ * Signals that the operation will result in duplicate Classes (Classes are considered duplicates if they have the same
+ * date).
+ */
+public class SessionNotFoundException extends RuntimeException{}


Is there a missing space between RunTimeException and "{}"?

marcustw · 2020-10-02T16:01:25Z

src/main/java/seedu/address/storage/JsonAdaptedAttributes.java

+/**
+ * Jackson-friendly version of {@link Attributes}.
+ */
+public class JsonAdaptedAttributes {


erisjacey

Amazing job! Just some minor nitpicks with regard to code styling (mostly just unnecessary line breaks/white spaces, a few naming alternatives to consider).

Stellar job nonetheless 😄 This is really top-notch effort.

erisjacey · 2020-10-03T04:57:21Z

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

+    public static final String COMMAND_WORD = "clearsession";
+    public static final String MESSAGE_SUCCESS = "The session in the current session list has been cleared";
+
+


Should this unncessary extra line break be removed?

erisjacey · 2020-10-03T05:04:17Z

src/main/java/seedu/address/logic/parser/CliSyntax.java

+    public static final Prefix PREFIX_SESSIONNAME = new Prefix("s/");
+    public static final Prefix PREFIX_SESSIONDATE = new Prefix("d/");


Maybe these could be renamed to PREFIX_SESSION_NAME and PREFIX_SESSION_DATE?

src/main/java/seedu/address/model/attendance/SessionDate.java

erisjacey · 2020-10-03T06:26:40Z

src/main/java/seedu/address/model/attendance/SessionList.java

+            if (sessions.size() != ((SessionList) other).getSessions().size()) {
+                return false;
+            }
+
+            Session[] testArray1 = new Session[sessions.size()];
+            Session[] testArray2 = new Session[sessions.size()];
+
+            int i = 0;
+            int j = 0;
+            for (Session s: sessions) {
+                testArray1[i] = s;
+                i++;
+            }
+            for (Session s: ((SessionList) other).getSessions()) {
+                testArray2[j] = s;
+                j++;
+            }
+
+            for (int k = 0; k < sessions.size(); k++) {
+                if (!testArray1[k].isSameSession(testArray2[k])) {
+                    return false;
+                }
+            }


Maybe some comments could be added here to more clearly explain the thought process?

erisjacey · 2020-10-03T06:33:15Z

src/main/java/seedu/address/storage/JsonAdaptedSession.java

+ */
+public class JsonAdaptedSession {
+
+


Unnecessary additional line break?

erisjacey · 2020-10-03T06:51:11Z

src/test/java/seedu/address/model/ModelManagerTest.java

@@ -18,6 +19,7 @@
 import seedu.address.model.person.NameContainsKeywordsPredicate;
 import seedu.address.testutil.AddressBookBuilder;

+


Unnecessary line break?

erisjacey · 2020-10-03T06:51:31Z

src/test/java/seedu/address/model/attendance/IndexRangeTest.java

+
+import org.junit.jupiter.api.Test;
+
+


Unnecessary line break?

erisjacey · 2020-10-03T06:51:54Z

src/test/java/seedu/address/model/attendance/SessionDateTest.java

+
+import org.junit.jupiter.api.Test;
+
+


Unnecessary line break?

erisjacey · 2020-10-03T06:54:32Z

src/test/java/seedu/address/storage/JsonAdaptedSessionTest.java

+
+

Unnecessary whitespaces?

erisjacey · 2020-10-03T06:54:45Z

src/test/java/seedu/address/storage/JsonSerializableSessionListTest.java

+
+
+
+


Unnecessary whitespaces?

CodingCookieRookie · 2020-10-03T16:54:26Z

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

    }

    @Override
    public void addPerson(Person person) {
+        for (Session s: sessionList.getSessions()) {


Should we remove this and use run in debug mode when its finalised

CodingCookieRookie · 2020-10-03T16:57:56Z

src/test/java/seedu/address/logic/commands/AddSessionCommandTest.java

+
+    @Test
+    public void execute_sessionAcceptedByModel_addSuccessful() throws Exception {
+        ModelStubAcceptingSessionAdded modelStub = new ModelStubAcceptingSessionAdded();


I like your usage of stubs!

CodingCookieRookie · 2020-10-03T17:04:47Z

src/main/java/seedu/address/model/attendance/IndexRange.java

+     * Returns if a given string is a valid index range.
+     */
+    public static boolean isValidIndexRange(String test) {
+        return test.matches(VALIDATION_REGEX_1) || test.matches(VALIDATION_REGEX_2);


Pattern.matches throw some exceptions when dealing with some corner cases like. Perhaps String.matches() do as well?

CodingCookieRookie · 2020-10-03T17:10:51Z

src/main/java/seedu/address/model/attendance/Session.java

@@ -17,16 +16,26 @@
 */
 public class Session implements Comparable<Session> {

-    private final LocalDate date;
+    private final SessionName sessionName;
+    private final SessionDate sessionDate;
    private final Map<Integer, Attributes> studentList;


Perhaps studentAttributesList would be better here?

…to v1.2.1-attendence

CodingCookieRookie

LGTM

YuunoKun added 4 commits October 2, 2020 22:01

Implement attendance features

73fa92a

Implement attdance features

aef39f6

Merge branch 'master' into v1.2.1-attendence

1450494

# Conflicts: # src/main/java/seedu/address/commons/core/Messages.java # src/main/java/seedu/address/logic/parser/AddressBookParser.java

Fix no newline EOF

519b513

YuunoKun added this to the v1.2.1 milestone Oct 2, 2020

This was linked to issues Oct 2, 2020

Track attendance progress #5

Closed

Take attendance #10

Closed

marcustw reviewed Oct 2, 2020

View reviewed changes

erisjacey reviewed Oct 3, 2020

View reviewed changes

Merge branch 'master' into v1.2.1-attendence

ebf8703

CodingCookieRookie reviewed Oct 3, 2020

View reviewed changes

YuunoKun added 2 commits October 4, 2020 12:21

Fix minor grammar issues and Strings

69105d6

Merge branch 'v1.2.1-attendence' of https://github.com/YuunoKun/tp in…

d278414

…to v1.2.1-attendence

CodingCookieRookie approved these changes Oct 4, 2020

View reviewed changes

CodingCookieRookie merged commit b013946 into AY2021S1-CS2103T-W16-4:master Oct 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v1.2.1 attendence #81

v1.2.1 attendence #81

YuunoKun commented Oct 2, 2020

marcustw left a comment

marcustw Oct 2, 2020

YuunoKun Oct 3, 2020

marcustw Oct 4, 2020

marcustw Oct 2, 2020

marcustw Oct 2, 2020

marcustw Oct 2, 2020

erisjacey left a comment

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

erisjacey Oct 3, 2020

CodingCookieRookie Oct 3, 2020

CodingCookieRookie Oct 3, 2020

CodingCookieRookie Oct 3, 2020

CodingCookieRookie Oct 3, 2020

CodingCookieRookie left a comment

		public static final String COMMAND_WORD = "clearsession";
		public static final String MESSAGE_SUCCESS = "The session in the current session list has been cleared";

		public static final Prefix PREFIX_SESSIONNAME = new Prefix("s/");
		public static final Prefix PREFIX_SESSIONDATE = new Prefix("d/");

		@@ -18,6 +19,7 @@
		import seedu.address.model.person.NameContainsKeywordsPredicate;
		import seedu.address.testutil.AddressBookBuilder;

v1.2.1 attendence #81

v1.2.1 attendence #81

Conversation

YuunoKun commented Oct 2, 2020

marcustw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erisjacey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodingCookieRookie left a comment

Choose a reason for hiding this comment