-
-
Notifications
You must be signed in to change notification settings - Fork 86
parallel-letter-frequency: add exercise #238
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
"name": "Parallel Letter Frequency", | ||
"uuid": "9ba6a56b-77bb-494b-88c2-0b751a1f66a9", | ||
"practices": [ | ||
"async", |
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 don't know what we want to call this concept?
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.
Processes or actors or OTP perhaps.
# Instructions append | ||
|
||
## Using Gleam OPT | ||
|
||
To compute the letter frequencies in parallel, you can use the [gleam_otp library](https://hexdocs.pm/gleam_otp/). | ||
This library is built to be compatible with Erlang's OTP actor framework and has many concurrency-related features. |
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.
Feel free to improve this text. I've added this to point students in the right direction.
[dependencies] | ||
gleam_stdlib = "~> 0.26" | ||
gleam_bitwise = "~> 1.2" | ||
gleam_otp = "~> 0.5" |
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 is a new dependency. If we're fine with that, I can add it to the test runner.
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 would prefer not to add this one, it is very rough and ready, lacks docs, and likely will undergo various breaking changes. I think it would be a frustrating experience for the student and not show the language in a good light to present this up front.
This exercise doesn't have canonical data right? |
It would be great to do that, but honestly I don't see the need for it happening before this PR could be merged. |
Just to be clear: I agree that adding canonical data would be great! I just feel like that should be a separate thing and not block this. I'll work on a prob-specs PR. |
[dependencies] | ||
gleam_stdlib = "~> 0.26" | ||
gleam_bitwise = "~> 1.2" | ||
gleam_otp = "~> 0.5" |
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 would prefer not to add this one, it is very rough and ready, lacks docs, and likely will undergo various breaking changes. I think it would be a frustrating experience for the student and not show the language in a good light to present this up front.
pub fn calculate_frequencies(texts: List(String)) -> Map(String, Int) { | ||
texts | ||
|> list.map(calculate_frequency) | ||
|> list.map(task.await(_, 100)) |
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 is a common pitfall: Waiting on tasks like this will potentially wait 0.1 * len(texts) seconds rather than 0.1 seconds.
An await_many is desired here, though one does not exist in the library yet.
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.
An await_many is desired here, though one does not exist in the library yet.
Yeah, I was looking for one but didn't find it.
"name": "Parallel Letter Frequency", | ||
"uuid": "9ba6a56b-77bb-494b-88c2-0b751a1f66a9", | ||
"practices": [ | ||
"async", |
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.
Processes or actors or OTP perhaps.
Would it then make sense to wait with merging this PR until async functionality is properly supported? I could convert this to a draft PR. |
Or maybe close it for now? |
Unfortunately I think it's best to delay this exercise. It's a nice one for showing the strengths of Gleam but the UX of learning that side of Gleam is really very poor at the moment. |
Sure |
No description provided.