Fix avail command bugs and add test cases #351

seanflyyy · 2022-11-06T09:25:34Z

Add ability to get next available date from the current time

Closes #297 #259 #258 #257 #250 #243 #236
Initial bugs from original PR #198

…ay class if current time past timeRange

Add basic test cases (not complete) Add UniqueStudentList revamped code (not complete) Update `TeachersPet::findAvailableClass` for the purpose of testing

Adds consideration of endTime if 12AM is stored as LocalTime.of(0, 0). As a result of this, the checks for the timeRange will always fail, and so I have written logic such that if the endTime is at 12AM, it will just `continue`.

…into fix-avail-command-bugs * 'master' of https://github.com/AY2223S1-CS2103T-T09-4/tp: Update java doc description for compare to methods Fix small ui initialization bug Remove spacing from 2 lines Refine parts of DG Fill in rest of PPP Fix inaccurate fields for Student Add javadoc comment Fix command messages Fix checkstyle bugs Add tests Fix student sharing same phone as NOK bug in edit command Fix student sharing same phone as NOK bug in add command Update uml diagrams to Teachers Pet Update png files to current project specifications Refactor uml diagrams to Teachers Pet Replace DG-images for student to person refactor

edit 1 dt/2022-11-06 0900-1000 edit 2 dt/2022-11-08 1000-1100 edit 3 dt/2022-11-07 1000-1100 avail 0900-1030 61 expect 2022-11-09 0900-1001 as return

edit 1 dt/2022-11-06 0900-1300 edit 2 dt/2022-11-06 1300-1600 avail 0800-1800 120 (returned 2022-11-06 1600-1800) edit 1 dt/2022-11-06 0900-1300 edit 2 dt/2022-11-06 1300-1600 edit 3 dt/2022-11-08 1000-1200 avail 0800-1800 120 (returned 2022-11-06 1600-1800)

Echomo-Xinyu

I am still reading the code now and have released the above comments so that you can work on them while I review.

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

Echomo-Xinyu · 2022-11-06T09:59:01Z

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

+                .filter(student -> student.getDisplayedClass().startTime != null
+                        && student.getDisplayedClass().endTime != null
+                        && student.getDisplayedClass().date != null
+                        && student.getDisplayedClass().date.compareTo(currDate) == 0)
+                .sorted(Student::compareToByClassAsc)
+                .map((element) -> element.getDisplayedClass())


After the merge of Caden's PR to remove displayClass field, this part needs to be changed accordingly.

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

[Missed Corner Case] At the current timing of "1730", with only one class set to be "2022-11-06 1630-1830", when calling the "avail 1630-1930 61", it will give a result of "Available date is: 2022-11-06 1830-1931" which is out of the given range.

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

Echomo-Xinyu · 2022-11-06T12:23:21Z

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

+                    if (endTimeFromTr.compareTo(classToCompare.startTime) <= 0) {
+                        newClass = new Class(currDate, startTimeFromTr, endTimeFromTr);
+                    }
+                } else if (currTime.compareTo(classToCompare.startTime) >= 0


the condition in line 617 can be directly derived from negation of line 613 and can be simplified later.

[Missed case regarding to time parsing]
with current time being 2024 and the only lesson is from 2000-2300 (ie including the current time),
when searching "avail 1700-2359 60", the result is "Available date is: 2022-11-06 2300-0000" which goes beyond the time range specified. This is regarding the incorrect handling of "0000" by endTimeFromClass

Echomo-Xinyu · 2022-11-06T12:41:44Z

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

+            } else if (classToCompare.endTime.compareTo(tr.endTimeRange) < 0
+                    && classToCompare.startTime.compareTo(tr.startTimeRange) > 0) {
+                if (currTime.compareTo(classToCompare.startTime) < 0) {
+                    if (endTimeFromTr.compareTo(classToCompare.startTime) <= 0) {


The comparison should be between endTimeFromCurrTime and classToCompare.startTime.

This will result the error when:
current time: 2040
only time 2100-2200
avail 2000-2300 10 will give Available date is: 2022-11-06 2000-2010 which is before the current time.

Echomo-Xinyu · 2022-11-06T12:53:28Z

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

+                if (currTime.compareTo(classToCompare.startTime) < 0) {
+                    if (endTimeFromTr.compareTo(classToCompare.startTime) <= 0) {
+                        newClass = new Class(currDate, startTimeFromTr, endTimeFromTr);
+                    }


[Missed corner case]
when the current time is before the single class start time, the front part is checked but the part after the class is ignored.
Current time: 2052
Single lesson: 2100-2200
avail 2030-2359 60 expects to give 2022-11-06 2200-2300 but gives 2022-11-07 2030-2130 in reality.

…into fix-avail-command-bugs * 'master' of https://github.com/AY2223S1-CS2103T-T09-4/tp: (23 commits) Improve method naming and error message Improve grammar Minor update for NFR in DG Improve format for UG Fix undo not resetting bug Fix style issues Add more tests for mark Edit existing tests Remove displayedClass field from all main code Remove automated next class feature Remove displayedClass field from Student Fix checkstyle Update message Display error message Prevent saving of files if json is in invalid format Fix Compile Error by UndoCommandParser Fix checkstyle Add new line Update javadocs Update testing and add new json file ...

Echomo-Xinyu · 2022-11-06T12:59:54Z

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

+                        newClass = new Class(currDate, startTimeFromCurrTime, endTimeFromCurrTime);
+                    }
+                }
+            } else {


this else block is duplicative of the nullity check in line 644. Same applies to the else block in line 639.

with current time being 2024 and the only lesson is from 2000-2300 (ie including the current time), when searching "avail 1700-2359 60", the result is "Available date is: 2022-11-06 2300-0000" which goes beyond the time range specified. This is regarding the incorrect handling of "0000" by endTimeFromClass

Bug is when current time: 2040 only time 2100-2200 avail 2000-2300 10 will give Available date is: 2022-11-06 2000-2010 which is before the current time.

edit 1 dt/2022-11-07 1300-1400 edit 2 dt/2022-11-07 1400-1500 avail 1300-1400 60 Incorrectly return 2022-11-07 2337-0037

Echomo-Xinyu · 2022-11-06T16:25:55Z

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

        }

+
+        // Link to design for more than 2 classes: https://arc.net/e/8D5B34DF-70C5-4A22-8D26-4BFC6CEEABC5
        for (int i = 0; i < list.size(); i++) {


As I am not done reading the code, so cannot really tell which part goes wrong. But I have found the bug here:
[Missed corner case]
current time: 0023
classes: [2022-11-07 0000-0100, 2022-11-07 0800-0900]
avail 0000-2300 100 expects 2022-11-07 0100-0240 but receives 2022-11-07 0023-0203

Sure, I'll look into it. In the future, there is no need to try to find the bug for me. I can trace the code to try to solve the bug! Thank you so much for pointing it out in the past though!

seanflyyy · 2022-11-06T16:34:52Z

Close #217

Echomo-Xinyu · 2022-11-06T16:47:50Z

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

        for (int i = 0; i < list.size(); i++) {
-            Class aFirstClass = list.get(i).getAClass();
+            Class aFirstClass = list.get(i);


[Missed corner case]
current time: 0045
classes: [2022-11-08 0000-0100, 2022-11-08 0400-0600]
avail 0000-0400 10 expects 2022-11-07 0045-0055 but receives 2022-11-08 0100-0110

[Missed corner case] current time: 0023 classes: [2022-11-07 0000-0100, 2022-11-07 0800-0900] avail 0000-2300 100 expects 2022-11-07 0100-0240 but receives 2022-11-07 0023-0203

[Missed corner case] current time: 0045 classes: [2022-11-08 0000-0100, 2022-11-08 0400-0600] avail 0000-0400 10 expects 2022-11-07 0045-0055 but receives 2022-11-08 0100-0110

Echomo-Xinyu · 2022-11-06T18:02:39Z

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

        for (int i = 0; i < list.size(); i++) {
-            Class aFirstClass = list.get(i).getAClass();
+            Class aFirstClass = list.get(i);
+


[Missed corner case]
current time: 0149
classes: [2022-11-08 0200-0300, 2022-11-08 0400-0500]
avail 0000-0400 15 expects 2022-11-07 0149-0204 but receives 2022-11-07 0000-0015

seanflyyy added 9 commits November 5, 2022 00:01

Add logic to take into consideration marked classes and giving next d…

dc81109

…ay class if current time past timeRange

Merge upstream into branch

24f4ddb

Add logic to handle no classes and give next day if the time has passed

5e65bde

Initial commit

3e14e7b

Add basic test cases (not complete) Add UniqueStudentList revamped code (not complete) Update `TeachersPet::findAvailableClass` for the purpose of testing

Fixes AY2223S1-CS2103T-T09-4#259 bug

8a4e09a

Adds consideration of endTime if 12AM is stored as LocalTime.of(0, 0). As a result of this, the checks for the timeRange will always fail, and so I have written logic such that if the endTime is at 12AM, it will just `continue`.

Fix previous class clash issue

c192af0

Fix bug where given the following days

34d3107

edit 1 dt/2022-11-06 0900-1000 edit 2 dt/2022-11-08 1000-1100 edit 3 dt/2022-11-07 1000-1100 avail 0900-1030 61 expect 2022-11-09 0900-1001 as return

Fix bugs

65287aa

edit 1 dt/2022-11-06 0900-1300 edit 2 dt/2022-11-06 1300-1600 avail 0800-1800 120 (returned 2022-11-06 1600-1800) edit 1 dt/2022-11-06 0900-1300 edit 2 dt/2022-11-06 1300-1600 edit 3 dt/2022-11-08 1000-1200 avail 0800-1800 120 (returned 2022-11-06 1600-1800)

seanflyyy requested a review from Echomo-Xinyu November 6, 2022 09:25

Echomo-Xinyu reviewed Nov 6, 2022

View reviewed changes

src/main/java/seedu/address/model/student/UniqueStudentList.java Show resolved Hide resolved

seanflyyy added 3 commits November 6, 2022 17:46

Fix checkstyle issues

d5c25f9

Correct spelling of word "situation"

8d7a97d

Fix missing corner case

a3be7a3

[Missed Corner Case] At the current timing of "1730", with only one class set to be "2022-11-06 1630-1830", when calling the "avail 1630-1930 61", it will give a result of "Available date is: 2022-11-06 1830-1931" which is out of the given range.

Echomo-Xinyu reviewed Nov 6, 2022

View reviewed changes

src/main/java/seedu/address/model/student/UniqueStudentList.java Show resolved Hide resolved

Fix if statement as it was being covered by previous if statement

569956a

Echomo-Xinyu reviewed Nov 6, 2022

View reviewed changes

seanflyyy added 8 commits November 6, 2022 22:12

Fixes bug.

6b055f6

Bug is when current time: 2040 only time 2100-2200 avail 2000-2300 10 will give Available date is: 2022-11-06 2000-2010 which is before the current time.

Removed duplicate else blocks

aac6193

Update retrieval of studentList to just be from internalList

7764312

Add UniqueScheduleList test cases

7cf628a

Add assertions based on IntelliJ recommendations

530e60c

Fix checkstyle issues

d9eaf29

Fix bug

5e03a3d

edit 1 dt/2022-11-07 1300-1400 edit 2 dt/2022-11-07 1400-1500 avail 1300-1400 60 Incorrectly return 2022-11-07 2337-0037

Echomo-Xinyu reviewed Nov 6, 2022

View reviewed changes

seanflyyy changed the title ~~Fix avail command bugs~~ Fix avail command bugs and add test cases Nov 6, 2022

Echomo-Xinyu reviewed Nov 6, 2022

View reviewed changes

seanflyyy added 2 commits November 7, 2022 00:57

Fix corner case

7f9f0ce

[Missed corner case] current time: 0023 classes: [2022-11-07 0000-0100, 2022-11-07 0800-0900] avail 0000-2300 100 expects 2022-11-07 0100-0240 but receives 2022-11-07 0023-0203

Fix corner case

4efa010

[Missed corner case] current time: 0045 classes: [2022-11-08 0000-0100, 2022-11-08 0400-0600] avail 0000-0400 10 expects 2022-11-07 0045-0055 but receives 2022-11-08 0100-0110

Echomo-Xinyu reviewed Nov 6, 2022

View reviewed changes

seanflyyy closed this Nov 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix avail command bugs and add test cases #351

Fix avail command bugs and add test cases #351

seanflyyy commented Nov 6, 2022

Echomo-Xinyu left a comment

Echomo-Xinyu Nov 6, 2022

Echomo-Xinyu Nov 6, 2022

Echomo-Xinyu Nov 6, 2022 •

edited

Loading

seanflyyy Nov 6, 2022

Echomo-Xinyu Nov 6, 2022

seanflyyy Nov 6, 2022

Echomo-Xinyu Nov 6, 2022

seanflyyy Nov 6, 2022

Echomo-Xinyu Nov 6, 2022 •

edited

Loading

seanflyyy Nov 6, 2022

Echomo-Xinyu Nov 6, 2022 •

edited

Loading

seanflyyy Nov 6, 2022

seanflyyy commented Nov 6, 2022

Echomo-Xinyu Nov 6, 2022

seanflyyy Nov 6, 2022

Echomo-Xinyu Nov 6, 2022

Fix avail command bugs and add test cases #351

Fix avail command bugs and add test cases #351

Conversation

seanflyyy commented Nov 6, 2022

Echomo-Xinyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Echomo-Xinyu Nov 6, 2022 • edited Loading

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

Echomo-Xinyu Nov 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Echomo-Xinyu Nov 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanflyyy commented Nov 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Echomo-Xinyu Nov 6, 2022 •

edited

Loading

Echomo-Xinyu Nov 6, 2022 •

edited

Loading

Echomo-Xinyu Nov 6, 2022 •

edited

Loading