-
Notifications
You must be signed in to change notification settings - Fork 357
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
Simplify fragments #1881
base: main
Are you sure you want to change the base?
Simplify fragments #1881
Conversation
# Conflicts: # modules/core/src/main/scala/doobie/util/fragments.scala # modules/core/src/test/scala/doobie/util/FragmentsSuite.scala
} | ||
|
||
test("Usage test: whereAndOpt") { | ||
assertEquals(whereAndOpt(Some(fr"hi"), orOpt(List.empty[Fragment]), orOpt(List(fr"a", fr"b"))).query[Unit].sql, "WHERE hi AND (a OR b ) ") |
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.
Do you think given this example is painful to work with? I feel like this is alright?
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.
Or course, this small example is fine as such. But in more complex queries, the options are more annoying to work with.
With this PR taken into account, you can express the same idea like this:
whereAnd(Some(fr"hi"), orOpt(List.empty[Option[Fragment]], onEmpty = true), or(List(fr"a", fr"b")))
|
Can we just revert back to what was in 1.0.0-RC2? It was simple, one method for each and no boolean parameters with default values... |
One specific example that I've just come across where returning sql"""
...
${Fragments.andOpt(emailIsNull, substring)}
...
""" You can't interpolate this fragment. It fails, because the result is of |
|
I've rethought the recent changes we've made in recent time to
fragments
. And I think I have good news: I think we can simplifyfragments
quite a lot.Reducible
s. Most of the combinators work sensibly even for the empty case, i.e.Foldable
sFragment
combinators that returnedOption[Fragment]
in the past, and they have been nothing but pain to work with. It's so so much better design to always return justFragment
fromFragment
combinators -- it's lower complexity and easier to work with for users.