Optics module with initial lens impl #251
Conversation
Optics module with initial lens impl
nomisRev
requested review from
raulraja,
pakoito,
JorgeCastilloPrz and
a team
Codecov Report
@@ Coverage Diff @@
## master #251 +/- ##
=========================================
Coverage ? 50.11%
Complexity ? 247
=========================================
Files ? 114
Lines ? 2558
Branches ? 293
=========================================
Hits ? 1282
Misses ? 1161
Partials ? 115
Continue to review full report at Codecov.
|
|
Praise be! |
folder structure
| object LensLaws { | ||
|
|
||
| inline fun <A, B, reified F> laws(lens: Lens<A, B>, aGen: Gen<A>, bGen: Gen<B>, funcGen: Gen<(B) -> B>, EQA: Eq<A>, EQB: Eq<B>, FA: Applicative<F>): List<Law> = listOf( | ||
| Law("getSet", { forAll(aGen, { a -> EQA.eqv(lens.set(lens.get(a))(a), a) }) }), |
There was a problem hiding this comment.
Could these please be separated into functions? They'll read better and you can test them individually.
There was a problem hiding this comment.
Thanks! I made the changes. Shall I also move it to kategory-test?
There was a problem hiding this comment.
@raulraja What do you think about merging all tests/laws into a single package? Right now it's supposed to be split per module (effects-tests, recursion-tests, optics-tests) but given that it's not a deployable artifact I don't see any problem making it depend on all the modules and contain all laws.
| inline fun <A, B, reified F> laws(lens: Lens<A, B>, aGen: Gen<A>, bGen: Gen<B>, funcGen: Gen<(B) -> B>, EQA: Eq<A>, EQB: Eq<B>, FA: Applicative<F>): List<Law> = listOf( | ||
| Law("getSet", { forAll(aGen, { a -> EQA.eqv(lens.set(lens.get(a))(a), a) }) }), | ||
| Law("setGet", { forAll(aGen, bGen, { a, b -> EQB.eqv(lens.get(lens.set(b)(a)), b) }) }), | ||
| Law("setIdempotent", { forAll(aGen, bGen, { a, b -> EQA.eqv(lens.set(b)(lens.set(b)(a)), lens.set(b)(a)) }) }), |
There was a problem hiding this comment.
Add a more descriptive name for the law, like: "Lens law: is set idempotent". We've been using these through the codebase and they've worked well.
| lens = tokenLens, | ||
| aGen = TokenGen, | ||
| bGen = Gen.string(), | ||
| funcGen = Gen.create<(String) -> String> { { it } }, |
There was a problem hiding this comment.
We already provide a generator for functions, called genFunAtoB IIRC.
|
@nomisRev Excellent stuff! Excited to have lenses in Kategory and you as maintainer! |
Seperated Lens laws into seperate law functions
|
@raulraja you and me both! |
* Provide nullable { } implementation based on DelimitedScope
* provide singleton object with reference check for nested nullable value.
* Provide extra test cases for nullable.kt
* remove check problems with detekt
* provide TODO for future fun interface
* reduce library visibility for API users.
* nextShift values doesn't need to be nested nullable value.
* Fix warning for parameter renaming for overridden method.
* Eliminate the shift call on non-null values
* move NullableBindSyntax.kt into private inner class for nullable.kt
* fix argument ending
* trigger GitHub actions
* trigger GitHub actions
I was unsure about adding Lens laws to kategory-test so I left them in optics for now.