Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add find_free_time command #63

Merged
merged 9 commits into from
Apr 4, 2024
Merged

feat: Add find_free_time command #63

merged 9 commits into from
Apr 4, 2024

Conversation

taufiq
Copy link

@taufiq taufiq commented Apr 3, 2024

This PR adds a find_free_time command that finds all students who do not have any module timings that clash with the specified timing.

Solves #45

Example command

find_free_time d/Wed st/1845 et/1900

taufiq added 5 commits April 3, 2024 15:14
This predicate checks that for a given time range, a student is free.
Adds a new command similar to `FindCommand` except it finds students who are free in the specified timing.
Wrong logic this just returned false for when a student is free at the specified timing.
A `FindFreeTimeCommandParser` to parse and crap out the `FindFreeTimeCommand` with the relevant `startTime` and `endTime` parsed
Whitespace separation was missing when stating how the `find_free_time` command is to be used.
@taufiq taufiq requested review from a team and removed request for a team April 3, 2024 07:38
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 0% with 56 lines in your changes are missing coverage. Please review.

Project coverage is 57.86%. Comparing base (0e07720) to head (c5bd7e4).
Report is 10 commits behind head on master.

❗ Current head c5bd7e4 differs from pull request most recent head cab20af. Consider uploading reports for the commit cab20af to get more accurate results

Files Patch % Lines
...a/seedu/address/model/student/IsFreePredicate.java 0.00% 25 Missing ⚠️
...du/address/logic/commands/FindFreeTimeCommand.java 0.00% 17 Missing ⚠️
...ddress/logic/parser/FindFreeTimeCommandParser.java 0.00% 13 Missing ⚠️
.../seedu/address/logic/parser/AddressBookParser.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master      #63      +/-   ##
============================================
- Coverage     59.64%   57.86%   -1.79%     
  Complexity      434      434              
============================================
  Files            91       94       +3     
  Lines          1819     1875      +56     
  Branches        178      186       +8     
============================================
  Hits           1085     1085              
- Misses          696      752      +56     
  Partials         38       38              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

taufiq added 2 commits April 3, 2024 16:32
Adds a 'd/' prefix to the `FindFreeTimeCommand` so that users can specify a day.
@taufiq taufiq requested a review from a team April 3, 2024 08:38
Copy link

@blaukc blaukc left a comment

Choose a reason for hiding this comment

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

Very cool PR, just one small nit on the compareTo part

Comment on lines 32 to 41
return student.getModuleTimings()
.stream()
.filter(
moduleTiming -> moduleTiming.getDay().equals(day)
)
.noneMatch(
moduleTiming ->
startTime.compareTo(moduleTiming.getEndTime()) == -1
&& endTime.compareTo(moduleTiming.getStartTime()) == 1
);
Copy link

Choose a reason for hiding this comment

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

Very cool use of streams! The compareTo part maybe should be < 0 instead of == -1 (viceversa for the ==1 part), according to Sam Patrick Floyd from StackOverflow

image

/**
* Tests that a {@code Student} is free between a given timing
*/
public class IsFreePredicate implements Predicate<Student> {
Copy link

Choose a reason for hiding this comment

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

Very cool usage of Predicate

@blaukc blaukc merged commit af80085 into master Apr 4, 2024
2 checks 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.

2 participants