-
-
Notifications
You must be signed in to change notification settings - Fork 112
Add new practice exercise swift-scheduling
#733
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Jeremie, I'm not sure about the solution to this exercise. What it is mainly doing is showing that it is stupidly hard in Elm to work with dates and times. Which isn't really true, most people just reach for one of the helper packages. I think we shouldn't give students the impression that this is how to work with dates in Elm, because it isn't what it makes sense to do in most situations.
Maybe we could add one of the common date package things instead to make the solution more ergnomic / idiomatic?
I think this is the one I normally use
https://package.elm-lang.org/packages/justinmimbs/date/latest/
| Quarter Int | ||
|
||
|
||
parseDescription : String -> Description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parsing code looks good, although its a shame that the exercise doesn't have any requirements about what to do when the string isn't formatted corectly.
|
||
type alias DateTime = | ||
{ year : Int | ||
, month : Month |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like the code would be simpler if the Month was an Int? You do lose a bit of type checking kind of, but all the other things are Ints anyway, and this is a practice exercise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going though Month
is unavoidable, since that's what Time.toMonth
gives you, but you are right that an Int
would have been a good choice too. I like the type safety so I want with this, but ultimately since this is only an example, that's entirely the student's choice.
@@ -387,52 +387,50 @@ | |||
"name": "Bob", | |||
"uuid": "11ce2a73-3f43-4b14-b755-6c52dc71bdda", | |||
"practices": [ | |||
"strings" | |||
"booleans" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it really makes sense for me to check all these, so maybe just let me know how confident you are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fairly confident :)
You say stupidly hard, I say stupidly fun :) That being said, you might be right. I don't think we should import a new package for this, so maybe we should simply chose not to implement this exercise? Would that be your recommendation? |
I think it would be less fun if you were panicking to meet a deadline :) It's a good point though, some people will be on exercism to try interesting challenges, whereas others will be trying to learn a new language as quickly as possible, so maybe we should cater to both. So maybe we keep the exercise but make it very obvious that its just for fun, and wouldn't be a real world thing, and that you would use a library instead? I think a Time concept is a good idea. Elm is very different to most other languages with this. But I think for the concept you would definitely want to show students the ergonomic / idiomatic way of doing things, which I think requires us to add whichever package we think is best. I'm open to suggestions though. |
Let's not bring real world stress to Exercism :)
Wise advice, I added an instruction append, let me know if that works.
I'm not sure we should use elm community packages for Exercism. The primary goal for Exercism is to teach languages, things like syntax and concepts. Once you've learned that, you should be able to use any package that's useful for your applications.If we start exploring packages, our task will never be done. |
I think this is a nice, concise point about elm/time:
Can we add it to the instructions.append? |
exercises/practice/swift-scheduling/.docs/instructions.append.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Jeremie, that change looks good, and I've proposed a change to the line below as well so that they work well together.
Cheers, Cedd
exercises/practice/swift-scheduling/.docs/instructions.append.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Cedd Burge <ceddlyburge@users.noreply.github.com>
This is a brand new exercise that looked interesting so I implemented it.
It's entirely based on dates, so since
Time
is so limited, I had to implement a lot of things myself, which was fun. The tests look a little strange, but I though spelling out the year and stuff would be a lot more useful than checking a Posix integer, it certainly helped me when I was solving it.I am now thinking that we could have a
time
concept, I think I'll open an issue. It would be pretty short.I also played around with
config.json
(part of #630). Maybe it could have been a separate PR...