Skip to content

Adding filterIf and filterOpt utility methods to Select #77

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

Merged
merged 3 commits into from
Mar 28, 2025

Conversation

NPCRUS
Copy link
Contributor

@NPCRUS NPCRUS commented Mar 27, 2025

fixes #63

@aboisvert
Copy link
Contributor

Code looks good overall. I just wonder about the ergonomics of filterOpt.

Thinking about some alternatives ...

// Currently proposed
ShippingInfo.select.filterOpt(someOpt)((value, table) => table.buyerId `=` value)

// Option 1: filterIf is all we need
ShippingInfo.select.filterIf(someOpt.isDefined)(_.buyerId `=` someOpt.get)

// Option 2: filterOpt doesn't provide unwrapped value
ShippingInfo.select.filterOpt(someOpt)(_.buyerId `=` someOpt.get)

// Option 3: Reverse the parameter order (table, value) instead of (value, table)
// (It seems to me table should come first also because it's the main object being 'carried over')
ShippingInfo.select.filterOpt(someOpt)(_.buyerId `=` _)

The aim is not necessarily code-golfing as it is to help produce easy-to-read queries.

And an even more drastic change would be to make Select a monad so it can be used in for-comprehensions and which could improve ergonomics but that's a more involved change that would require some experimentation to get the combinators right.

@NPCRUS
Copy link
Contributor Author

NPCRUS commented Mar 27, 2025

@aboisvert you are right, order in option 3 is better

@lihaoyi
Copy link
Member

lihaoyi commented Mar 27, 2025

I think this looks fine, happy to merge it when you are done

@NPCRUS
Copy link
Contributor Author

NPCRUS commented Mar 28, 2025

@lihaoyi im happy and could you pls make a minor release?

@lihaoyi lihaoyi merged commit b505d60 into com-lihaoyi:main Mar 28, 2025
6 checks passed
@lihaoyi
Copy link
Member

lihaoyi commented Mar 28, 2025

Tagged 0.1.17

@NPCRUS NPCRUS deleted the select-utility-methods branch April 3, 2025 13:46
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.

Values clause does not support empty sequence
3 participants