Skip to content

Issue#2: Implement Scheduler algorithm#9

Merged
mingyeonggg merged 5 commits into
mainfrom
iss2_kmg
Dec 11, 2025
Merged

Issue#2: Implement Scheduler algorithm#9
mingyeonggg merged 5 commits into
mainfrom
iss2_kmg

Conversation

@mingyeonggg

Copy link
Copy Markdown
Collaborator

Implemented logic to generate timetables by combining mandatory and optional courses, and added unit tests.

@mingyeonggg mingyeonggg changed the title Feat: Implement Scheduler algorithm Issue#2: Implement Scheduler algorithm Dec 11, 2025

@AyeongKwon AyeongKwon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!! 😎
Very nice logic to implement scheduling algorithm!

Comment thread src/main/java/com/unitime/algorthm/Scheduler.java Outdated
@mingyeonggg mingyeonggg self-assigned this Dec 11, 2025

@Kimhyewon0621 Kimhyewon0621 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make these tests easier to understand for other people, could you add some comments explaining the scenario for each test?

For example, in testTimeConflict, a comment like
// Verifying that a course with overlapping time is NOT added

@mingyeonggg

Copy link
Copy Markdown
Collaborator Author

Hello Hyewon!
That's a great suggestion!💝 I agree that adding scenario comments will significantly improve file easier to understand.
I've added some comments to Scheduler.java and SchedulerTest.java explaining its purpose. 😊😊

@Kimhyewon0621

Copy link
Copy Markdown
Owner

I check your comments! LGTM

@mingyeonggg mingyeonggg merged commit 917e92f into main Dec 11, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants