milewski-ctfp-pdf icon indicating copy to clipboard operation
milewski-ctfp-pdf copied to clipboard

Desugar scala code snippet 23 in chapter 3.4 as per issue #296

Open amacmillanparks opened this issue 2 years ago • 3 comments

As per issue #296, the Scala edition snippets 22 and 23 are identical when snippet 23 should be a desugared version of snippet 22. This PR fixes that.

amacmillanparks avatar Sep 15 '22 12:09 amacmillanparks

Hi,

Thanks for contributing!

This issue seems legit to me, but as I don't have much knowledge in Scala, I'll wait to have more feedback from any other contributor that can help in here.

drupol avatar Sep 15 '22 12:09 drupol

Thanks @drupol! Some colleagues and I have been reading through the book and just noticed this in the last chapter we read.

The change should be pretty self explanatory, but happy to discuss more if it'd help.

amacmillanparks avatar Sep 21 '22 13:09 amacmillanparks

Hi,

Thanks ! Ok then it's fine for me.

@hmemcpy Ok for you as well ?

drupol avatar Sep 21 '22 13:09 drupol

@hmemcpy ^^

drupol avatar Feb 01 '23 11:02 drupol

Oh wow, sorry for missing this. It looks fine, though I'd prefer to use the more idiomatic .flatMap style (with the dot, instead of infix).

Either way this should be fine.

hmemcpy avatar Feb 01 '23 12:02 hmemcpy

@amacmillanparks Are you planning to implement the changes requested by @hmemcpy ?

drupol avatar Feb 01 '23 12:02 drupol

Hi @drupol & @hmemcpy, sorry, I've just seen this now. Thanks for the feedback, I'll go ahead and make the change.

amacmillanparks avatar Feb 09 '23 11:02 amacmillanparks

That's all done now, please can you re-review?

amacmillanparks avatar Feb 09 '23 12:02 amacmillanparks

The CI has spoken but I'll wait for @hmemcpy to review this !

drupol avatar Feb 09 '23 12:02 drupol

Sorry, one more comment: since we're using the native flatMap method (and not the bind syntax >>=), the import bindSyntax._ line can also be removed.

hmemcpy avatar Feb 09 '23 14:02 hmemcpy

No worries, that's all done now.

amacmillanparks avatar Feb 09 '23 14:02 amacmillanparks

Fantastic, thanks very much for your contribution! And sorry it took a while ;)

hmemcpy avatar Feb 09 '23 14:02 hmemcpy