Add Monoidal (->) tests #43
Conversation
![hint-man[bot]](https://avatars.githubusercontent.com/in/20170?s=60&v=4){kind=small}
| prismSpecs = describe "Prism" $ do | ||
| describe "preview" $ do | ||
| xit "preview _Ctor x ≡ case (Ctor _) of" $ | ||
| $(inspectTest $ 'matchMarkPrism === 'matchMarkManual) `shouldSatisfy` isSuccess |
There was a problem hiding this comment.
Warning: Redundant do
describe "preview" $
do xit "preview _Ctor x \8801 case (Ctor _) of" $
$( inspectTest $ 'matchMarkPrism === 'matchMarkManual )
`shouldSatisfy` isSuccess
| prismSpecs = describe "Prism" $ do | ||
| describe "preview" $ do | ||
| xit "preview _Ctor x ≡ case (Ctor _) of" $ | ||
| $(inspectTest $ 'matchMarkPrism === 'matchMarkManual) `shouldSatisfy` isSuccess |
There was a problem hiding this comment.
Warning: Redundant do
xit "preview _Ctor x \8801 case (Ctor _) of" $
$( inspectTest $ 'matchMarkPrism === 'matchMarkManual )
`shouldSatisfy` isSuccess
| profunctorsSpec = describe "Profunctor" $ do | ||
| describe "(->)" $ do | ||
| it "Identity: dimap id id ≡ id" $ hedgehog $ do | ||
| f <- forAllWith (const "f") genFunction |
There was a problem hiding this comment.
Suggestion: Reduce duplication
| f <- forAllWith (const "f") genFunction | |
| Combine with out/test/prolens/Test/Prolens/Property.hs:75:13 |
| profunctorsSpec = describe "Profunctor" $ do | ||
| describe "(->)" $ do | ||
| it "Identity: dimap id id ≡ id" $ hedgehog $ do | ||
| f <- forAllWith (const "f") genFunction |
There was a problem hiding this comment.
Suggestion: Reduce duplication
| f <- forAllWith (const "f") genFunction | |
| Combine with out/test/prolens/Test/Prolens/Property.hs:80:13 |
| it "Composition: dimap (ab . bc) (yz . xy) ≡ dimap bc yz . dimap ab xy" $ hedgehog $ do | ||
|
|
||
| f <- forAllWith (const "f") genFunction | ||
| ab <- forAllWith (const "ab") genFunction |
There was a problem hiding this comment.
Suggestion: Reduce duplication
| ab <- forAllWith (const "ab") genFunction | |
| Combine with out/test/prolens/Test/Prolens/Property.hs:57:13 |
| it "Associativity" $ hedgehog $ do | ||
| f <- forAllWith (const "f") genFunction | ||
| x <- forAll genInt | ||
| dimap id id f x === f x |
There was a problem hiding this comment.
Warning: Redundant do
describe "(->)" $
do it "Identity: x -> id \8801 x" $
hedgehog $
do f <- forAllWith (const "f") genFunction
x <- forAll genInt
dimap id id f x === f x
it "Associativity" $
hedgehog $
do f <- forAllWith (const "f") genFunction
x <- forAll genInt
dimap id id f x === f x
|
Should I fix all hint-man comments? |
test/Test/Prolens/Property.hs
Outdated
| f <- forAllWith (const "f") genFunction | ||
| x <- forAll genInt | ||
| y <- forAll genInt | ||
| pappend pempty f (x, y) === (x, f y) |
There was a problem hiding this comment.
id can't be used, and you actually check another law here, which is correct 👍🏼
However, we can make it even a bit more elegant and general, if we use the fact that it is Strong. We can check these properties instead:
pappend f pempty ≡ first f
pappend pempty f ≡ second f| x <- forAll genInt | ||
| y <- forAll genInt | ||
| z <- forAll genInt | ||
| pappend f (pappend g h) (x, (y, z)) === (f x, (g y, h z)) |
There was a problem hiding this comment.
This looks right.
I think we could make it even more cooler, if we would check both associativity at once as well:
pappend f (pappend g h) (x, (y, z)) === dimap (\(a, (b, c)) -> ((a, b), c)) (\((a, b), c) -> (a, (b, c)))(pappend (pappend f g) h (x, (y, z)) )What do you think?
There was a problem hiding this comment.
I think that line becomes quite long.
My counterproposal is to add two helper functions
untupleRight :: (a, (b, c)) -> (a, b, c)
untupleRight (a, (b, c)) = (a, b, c)
untupleLeft :: ((a, b), c) -> (a, b, c)
untupleLeft ((a, b), c) = (a, b, c)And the test would become
it "Associativity" $ hedgehog $ do
f <- forAllWith (const "f") genFunction
g <- forAllWith (const "g") genFunction
h <- forAllWith (const "h") genFunction
x <- forAll genInt
y <- forAll genInt
z <- forAll genInt
untupleRight (pappend f (pappend g h) (x, (y, z))) === untupleLeft (pappend (pappend f g) h ((x, y), z))There was a problem hiding this comment.
Sure, the helper functions are way to go.
I don't think that dimap is too long to type, so I don't see why it is a problem..
There was a problem hiding this comment.
To be honest I can't get it to compile with dimap. I'm always running into issues like
Couldn't match type ‘Int’ with ‘((Int, b1), c0)’
Expected type: (Int, (Int, Int))
-> (((a0, b0), Int), ((Int, b1), c0))
Actual type: ((Int, Int), Int) -> ((Int, Int), Int)
Maybe it has something to do with the tuple type parameters?
There was a problem hiding this comment.
Okay, no worries. I would look into that separately 🙂
Address identity comment
Co-authored-by: Veronika Romashkina <[email protected]>
| x <- forAll genInt | ||
| y <- forAll genInt | ||
| z <- forAll genInt | ||
| pappend f (pappend g h) (x, (y, z)) === (f x, (g y, h z)) |
There was a problem hiding this comment.
Okay, no worries. I would look into that separately 🙂
Helps with #7
I'm not sure if I understood the laws correctly and how they apply here. Any comments are appreciated.