search

scalawithcats / scala-with-cats

Source code for Scala with Cats
http://underscore.io/books/scala-with-cats
395 stars 131 forks source link

CRDT excercise confusion #100

Open oker1 opened 6 years ago

oker1 commented 6 years ago

I'm not sure if this is worth a clarification, but I've had a bit of head scratching with this one.

If you create the BoundedSemiLattice[Int] instance in a worksheet outside the BoundedSemiLattice companion object, it'll get picked up as Monoid[Int] by the increment method. The increment method will not work properly. While I really liked this excercise, it can be a bit confusing since it requires a Monoid[Int] with the add behavior for increment and it requires a BoundedSemiLattice[Int] (which happens to be also a Monoid[Int]) with the max behavior for the merge operation.

davegurnell commented 6 years ago

Hey Zsolt,

Thanks for the issue. I agree that the case study needs some work.

@noelwelsh has a bunch of changes incoming that will hopefully address the problems!

Cheers, Dave

On Tue, Dec 5, 2017 at 10:58 AM Zsolt Takács notifications@github.com wrote:

I'm not sure if this is worth a clarification, but I've had a bit of head scratching with this one.

If you create the BoundedSemiLattice[Int] instance in a worksheet outside the BoundedSemiLattice companion object, it'll get picked up as Monoid[Int] by the increment method. The increment method will not work properly. While I really liked this excercise, it can be a bit confusing since it requires a Monoid[Int] with the add behavior for increment and it requires a BoundedSemiLattice[Int] (which happens to be also a Monoid[Int]) with the max behavior for the merge operation.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/underscoreio/advanced-scala/issues/100, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOI5_rpecgIamZly2V6XQvf-Nak435wks5s9SHlgaJpZM4Q2Gkd .

ioanluca commented 4 years ago

Hi everyone.

Any idea if we can change the signatures such that the BoundedSemiLattice instance for Int is picked up for merge and the Monoid instance for increment and total?