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

Add swift-scheduling exercise #2536

Merged
merged 19 commits into from
Mar 3, 2025
Merged

Add swift-scheduling exercise #2536

merged 19 commits into from
Mar 3, 2025

Conversation

ErikSchierboom
Copy link
Member

This is a proposal to add new date-based exercise. The primary goal is to have more date-based exercises. This exercise requires students to convert strings to dates, and will need to add date arithmetic. I'm open to any improvements, so please consider this a starting point.

As a PoC, I've implemented it in C#: exercism/csharp#2397

@ErikSchierboom ErikSchierboom requested a review from a team as a code owner March 1, 2025 13:43
Copy link
Member

@tasxatzial tasxatzial left a comment

Choose a reason for hiding this comment

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

Only reviewed docs

ErikSchierboom and others added 6 commits March 1, 2025 16:56
Co-authored-by: Anastasios Chatzialexiou <[email protected]>
Co-authored-by: Anastasios Chatzialexiou <[email protected]>
Co-authored-by: Anastasios Chatzialexiou <[email protected]>
Co-authored-by: Anastasios Chatzialexiou <[email protected]>
Co-authored-by: Anastasios Chatzialexiou <[email protected]>
Co-authored-by: Anastasios Chatzialexiou <[email protected]>
"meetingStart": "2023-04-19T00:00:00",
"description": "Q4"
},
"expected": "2023-12-29T08:00:00"
Copy link
Member

Choose a reason for hiding this comment

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

How about a test case that wraps around to the next year?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like that idea. I think it makes the most sense to have that as part of the ASAP tests. I'll add it

Copy link
Member Author

Choose a reason for hiding this comment

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

On second hand, the quarters suggestion is good! I've updated the description, I'll add the test cases later. Could you check if the description makes sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added test cases too

@ErikSchierboom
Copy link
Member Author

I don't know if anyone has time for it, but it would be great if someone could build an implementation to verify the test cases are correct.

@BethanyG
Copy link
Member

BethanyG commented Mar 1, 2025

I don't know if anyone has time for it, but it would be great if someone could build an implementation to verify the test cases are correct.

It'll take me a bit - but I can take a stab at doing one for Python.

| `"NOW"` | - | Two hours after the meeting started |
| `"ASAP"` | Before 12:00 | Today at 17:00 |
| `"ASAP"` | After 12:00 | Tomorrow at 12:00 |
| `"EOW"` | Monday, Tuesday, or Wednesday | Friday at 5:00 |
Copy link
Member

Choose a reason for hiding this comment

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

5am or 5pm?

Copy link
Member

Choose a reason for hiding this comment

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

The tests say 17:00

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

"meetingStart": "2001-04-09T09:00:00",
"description": "Q4"
},
"expected": "2001-12-28T08:00:00"
Copy link
Member

Choose a reason for hiding this comment

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

My implementation says 2001-12-31 is the last workday of this quarter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, fixed

@IsaacG
Copy link
Member

IsaacG commented Mar 1, 2025

I don't know if anyone has time for it, but it would be great if someone could build an implementation to verify the test cases are correct.

https://gist.github.com/IsaacG/08211c0b4769419e297f3b898a07d6dc

@ErikSchierboom
Copy link
Member Author

I've updated!

Comment on lines 23 to 24
| `"ASAP"` | Before 12:00 | Today at 17:00 |
| `"ASAP"` | After 12:00 | Tomorrow at 12:00 |
Copy link
Member

Choose a reason for hiding this comment

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

I think people might complain that it's unclear how to categorize exactly 12:00

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair point! I'll update to a less ambiguous time.

Copy link
Member

Choose a reason for hiding this comment

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

I meant that between the two categories "Before 12:00" and "After 12:00", the time "exactly at 12:00" isn't contained in either of them 😁

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! I also think that 12:00 might be ambiguous between AM and PM. I'll tweak it a bit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated

@angelikatyborska
Copy link
Member

I implemented the exercise in Elixir: https://github.com/exercism/elixir/pull/1551/files#diff-a0012d9d95c9713c1480d9f5ae9408ade5ab4bd412a65f3d7b69ed5d880e298d

All the tests made sense and I had a great time solving the exercise. I think students will be very happy to practice date times in a more advanced way ❤️

@IsaacG
Copy link
Member

IsaacG commented Mar 2, 2025

» python scheduling.py canonical-data.json
PASS: Test 0
PASS: Test 1
PASS: Test 2
PASS: Test 3
PASS: Test 4
PASS: Test 5
PASS: Test 6
PASS: Test 7
PASS: Test 8
PASS: Test 9
PASS: Test 10
PASS: Test 11
PASS: Test 12
PASS: Test 13

I think the data looks reasonable ;)

@ErikSchierboom
Copy link
Member Author

I've updated noon (12:00) to 13:00 to reduce any possible confusion between AM and PM and I've made ASAP use before and after or at (which was previously unspecified). Maybe one final check?

@IsaacG
Copy link
Member

IsaacG commented Mar 2, 2025

I've updated noon (12:00) to 13:00 to reduce any possible confusion between AM and PM and I've made ASAP use before and after or at (which was previously unspecified). Maybe one final check?

https://gist.github.com/IsaacG/e7e628532af521a505b3336ee0a09458

My updated solution seems to work fine with the test data.

@ErikSchierboom
Copy link
Member Author

Just needs two approvals then 🙂

@BethanyG
Copy link
Member

BethanyG commented Mar 2, 2025

Just needs two approvals then 🙂

It would appear that it needs maintainers-admin (codeowners).
Python implementation here: exercism/python#3876

I might have the Python version return the delivery dates in a format different from the input ISO, in order to have a little practice with datetime.strftime() and its format codes. Still pondering that.

Also wondering if it would be rude to include leap year scenarios or not.....

@ErikSchierboom
Copy link
Member Author

I might have the Python version return the delivery dates in a format different from the input ISO, in order to have a little practice with datetime.strftime() and its format codes. Still pondering that.

That sounds totally reasonable.

Also wondering if it would be rude to include leap year scenarios or not.....

Not at all. I'll add such a test case.

@ErikSchierboom
Copy link
Member Author

@BethanyG I've added a test case for a leap day: 2a5a746 Could someone verify its correctness?

@ErikSchierboom ErikSchierboom merged commit a8ca9c5 into main Mar 3, 2025
7 checks passed
@ErikSchierboom ErikSchierboom deleted the swift-scheduling branch March 3, 2025 18:05
@BethanyG
Copy link
Member

BethanyG commented Mar 3, 2025

Apologies for the delay, @ErikSchierboom -- test case validated. 😄

exercism/python#3876

@ErikSchierboom
Copy link
Member Author

No problem. Thanks for the validation

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.

6 participants