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

Support inclusive/exclusive ranges for sorted sets #35 #51

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

72squared
Copy link
Collaborator

No description provided.

Copy link
Owner

@scoquelin scoquelin left a comment

Choose a reason for hiding this comment

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

minor, but left some suggestions to avoid some previous workarounds

* @param inclusive Whether the boundary is inclusive. default is false.
* @tparam T The value type
*/
case class Boundary[T](value: Option[T] = None, inclusive: Boolean = false)
Copy link
Owner

@scoquelin scoquelin Sep 4, 2024

Choose a reason for hiding this comment

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

After making Boundary type argument covariant [+T], this basically allows Boundary(Option[Nothing]) (which is basically the unbounded case) to be used together with other Boundary(Option[T]) and you can get rid of all the new Boundary(...) and new ZRange(...) in the code below

* @tparam T The value type
* @return The boundary
*/
def unbounded[T]: Boundary[T] = new Boundary[T](None)
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
def unbounded[T]: Boundary[T] = new Boundary[T](None)
def unbounded[T]: Boundary[T] = Boundary(None)

@72squared 72squared merged commit e45559c into main Sep 5, 2024
3 checks passed
@72squared 72squared deleted the jl-zrange-inclusive branch September 5, 2024 12:59
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