nusmodifications / nusmods

🏫 Official course planning platform for National University of Singapore.
https://nusmods.com
MIT License
575 stars 300 forks source link

Add additional check for findExamClashes() for non-identical start times #3560

Closed EssWhyy closed 6 months ago

EssWhyy commented 1 year ago

Context

Fix issue #3449

Implementation

Per the discussion in the issue, I added in an extra check in findExamClashes() in timetables.ts. The steps for this check are as follows:

  1. Group modules in timetable by exam date only
  2. For each array of modules[] assigned to an exam date, we do a brute force O(n^2) check on each unique pair of modules
  3. If two modules have the same exam start date and time, then we ignore since its already accounted for in the existing clash check
  4. If two modules with different exam start time has clash based on time intervals, then we add it into the existing clashes dictionary via the dateTime string key

The result can be seen here for both test cases:

case1 case2

Other Information

The current exam clash warning text can only show one date & time, as such for two exams with different start times, I used the earlier start time for the warning text as a workaround for now.

The function could probably be less verbose and more efficient. But on testing my end, there are no major slowdowns on the performance when adding or deleting a typical number of modules to the timetable.

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nusmods-export ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 19, 2024 3:56pm
nusmods-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 19, 2024 3:56pm
vercel[bot] commented 1 year ago

@EssWhyy is attempting to deploy a commit to a Personal Account owned by @nusmodifications on Vercel.

@nusmodifications first needs to authorize it.

codecov[bot] commented 6 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (84f4c5a) 53.20% compared to head (a48a423) 53.48%.

Files Patch % Lines
website/src/utils/timetables.ts 92.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3560 +/- ## ========================================== + Coverage 53.20% 53.48% +0.27% ========================================== Files 272 272 Lines 5885 5931 +46 Branches 1397 1408 +11 ========================================== + Hits 3131 3172 +41 - Misses 2754 2759 +5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.