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 circular-buffer exercise #95

Merged
merged 1 commit into from
Sep 16, 2024
Merged

Conversation

ageron
Copy link
Contributor

@ageron ageron commented Sep 15, 2024

I had a bit of trouble getting the tests to compile due to Roc compiler issues, see:

I found workarounds, but they make the tests a bit ugly. Once these issues are fixed, I'll update this exercise:

  • I'll use |> Ok instead of Ok result.
  • I'll move all runOperations<N> functions inside their respective expect statement.

@ageron
Copy link
Contributor Author

ageron commented Sep 15, 2024

It might be a good idea to implement a solution that supports any value type (without changing the stub annotations, they can just continue to return I64 to keep things simple).
Edit: I just implemented this change on my machine, it was pretty straightforward:

  • Change CircularBuffer to:
CircularBuffer a : { data : List [Value a, Undefined], start : U64, length : U64 }
  • Change empty to:
{ data: List.repeat Undefined capacity, start: 0, length: 0 }
  • In read, handle Ok (Value value) and Ok Undefined:
         when data |> List.get start is
            Ok (Value value) -> Ok { newBuffer, value }
            Ok Undefined -> crash "Unreachable: we should never read an Undefined value"
            Err OutOfBounds -> crash "Unreachable: start should never be out of bounds"
  • In write and overwrite, use List.replace index (Value value) instead of List.replace index value.

That's it!

Do you think I should push this change? Again, it doesn't affect the stub, which is what 99% users see, so this change would just help the 1% of users who will look at this Example.roc. However, it might also make life a slightly harder for the track maintainers. Wdyt?

@Anton-4
Copy link
Contributor

Anton-4 commented Sep 16, 2024

Let\s keep it like this; in case of doubt, keep it simple :)

Copy link
Contributor

@isaacvando isaacvando left a comment

Choose a reason for hiding this comment

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

Looks great!

This makes me think that we should have an exercise somewhere that shows off opaque types. I don't think we should change this solution because it already looks like it was tricky enough to get working, but opaque types could be nice for data structures like this because they allow the implementation details to be hidden and force the user to use the provided API.

@ageron ageron merged commit 92f0f77 into exercism:main Sep 16, 2024
4 checks passed
@ageron ageron deleted the add-circular-buffer branch September 16, 2024 19:39
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.

3 participants