Skip to content
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

Assorted typos, suggestions, misc. for Part I #80

Closed
10 of 14 tasks
lJoublanc opened this issue Aug 16, 2017 · 7 comments
Closed
10 of 14 tasks

Assorted typos, suggestions, misc. for Part I #80

lJoublanc opened this issue Aug 16, 2017 · 7 comments

Comments

@lJoublanc
Copy link

lJoublanc commented Aug 16, 2017

Part I

  • 2.5.5 or pg 261 - Wouldnt it make more sense to use a Semigroup and NonEmptyList instead of Monoid? Having an empty Order doesnt make much sense.
  • Table on p. 50 type variance : if type is invariant, surely the type class variance does not inherit (column 1 in the table should be no/no). Also, example with Option/Some demonstrates this.
  • p. 81 Diagram supposed to show many-to-one flatMap is actually many-to-many, contrary to the description.
  • Suggestion for section on eval monad. Use call to 'current time' instead of a constant, in order to demonstrate differences between models.
  • p. 110 : "We need to have a monoid in scope", however, the example imports applicative instead.
  • Writer monad exercise crashes, when sequencing futures (no idea why):
    Welcome to Scala 2.12.3 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_101)
java.lang.NoClassDefFoundError: Could not initialize class $line13.$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$
        at scala.runtime.java8.JFunction0$mcI$sp.apply(JFunction0$mcI$sp.java:12)
        at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:653)
        at scala.util.Success.$anonfun$map$1(Try.scala:251)
        at scala.util.Success.map(Try.scala:209)
        at scala.concurrent.Future.$anonfun$map$1(Future.scala:287)
        at scala.concurrent.impl.Promise.liftedTree1$1(Promise.scala:29)
        at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:29)
        at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:60)
        at scala.concurrent.impl.ExecutionContextImpl$AdaptedForkJoinTask.exec(ExecutionContextImpl.scala:140)
        at java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(Unknown Source)
        at java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
        at java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
java.lang.NoClassDefFoundError: Could not initialize class $line13.$read$$iw$$iw$$iw$$iw$$iw$$iw$$iw$$iw$
        at scala.runtime.java8.JFunction0$mcI$sp.apply(JFunction0$mcI$sp.java:12)
        at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:653)
        at scala.util.Success.$anonfun$map$1(Try.scala:251)
        at scala.util.Success.map(Try.scala:209)
        at scala.concurrent.Future.$anonfun$map$1(Future.scala:287)
        at scala.concurrent.impl.Promise.liftedTree1$1(Promise.scala:29)
        at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:29)
        at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:60)
        at scala.concurrent.impl.ExecutionContextImpl$AdaptedForkJoinTask.exec(ExecutionContextImpl.scala:140)
        at java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(Unknown Source)
        at java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
        at java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
java.util.concurrent.TimeoutException: Futures timed out after [5 seconds]
  at scala.concurrent.impl.Promise$DefaultPromise.ready(Promise.scala:255)
  at scala.concurrent.impl.Promise$DefaultPromise.result(Promise.scala:259)
  at scala.concurrent.Await$.$anonfun$result$1(package.scala:215)
  at scala.concurrent.BlockContext$DefaultBlockContext$.blockOn(BlockContext.scala:53)
  at scala.concurrent.Await$.result(package.scala:142)
  ... 42 elided
  • page 120 "we can test that them"
  • page 121 "a computation of a result" --> "result of a computation" ?
  • page 125 parentheses missing "we can evaluate ( 1 + 2) * 3"
  • page 128 "should recusively call itself as long as the result of f returns a Right" but the example implies this should be Left
  • page 276 If tailRecM is not tail-recursive, will it still be stack-safe? Explain this.
  • Cartesian chapter : needs to be updated for 1.0.0, where builder |@| has been removed. Also pg. 183.
  • pg 185 7.2.2.3 "gere's an example"
  • pg 186 "OF"
@lJoublanc
Copy link
Author

Apologies for putting this all under one issue no. I'd made the list as I was going through the book and only now realised there was a git repo. Going forward I will raise issues individually.

@davegurnell
Copy link
Contributor

davegurnell commented Sep 11, 2017

No worries! Thanks very much for your help!

I'm doing bug fixes on the feature/bugfixes branch right now (well... tomorrow at this rate).

I'll edit the original issue description to include checkboxes and work through it. If there's anything I can't fix immediately, I'll raise another issue.

davegurnell pushed a commit that referenced this issue Sep 12, 2017
@davegurnell
Copy link
Contributor

Hi @lJoublanc. Thanks very much - I've done a first pass on your points in 5454487. There's loads of good feedback in there! I've put some comments below where I either had further questions, wanted to elaborate further, or disagreed with a point and wanted to explain why:

2.5.5 or pg 261 - Wouldnt it make more sense to use a Semigroup and NonEmptyList instead of Monoid? Having an empty Order doesn't make much sense.

I'm not sure I agree with this one. IMO the natural zero for an order from, say, Amazon, is to not buy anything: 0 quantity, 0 price.

p. 81 Diagram supposed to show many-to-one flatMap is actually many-to-many, contrary to the description.

The diagram looks ok to me, but it's probably down to interpretation. Is your issue the number of circles and stars in the lists on the left, middle, and right?

I've kind of mixed some metaphors here. On the one hand, the number of items in the list icon is supposed to be insignificant: three icons are meant to simply represent "a list of some length".

However, I've often found in training sessions that people get confused between the List[B] returned by the flatMap function and the List[B] returned by flatMap itself. I removed a star from the former of the two to clarify that the two lists are not the same. But now it looks like the number of stars has a significance, so I guess I should add a fourth star to the right to indicate that the lists can get longer.

What do you think?

Writer monad exercise crashes, when sequencing futures (no idea why):
Welcome to Scala 2.12.3 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_101)

This looks suspiciously like a binary compatibility issue. Make sure you're using compatible versions of Scala and Cats. I'll add a section to the start of the book on versions, but I don't think it'll help with your situation here.

page 276 If tailRecM is not tail-recursive, will it still be stack-safe? Explain this.

My solution here was actually completely wrong. It is possible to make this method tail-recursive---it's just tricky to write. You have to use a List as an explicit stack to replace the call stack. I've updated the text to:

  • include the non-tail-recursive solution and the tail-recursive solution;
  • add a short discussion on the effects of non-tail-recursive tailRecM
    (which are basically that Cats becomes non-stack-safe).

pg 186 "OF"

I can't see this one. I think it's gone in the latest text. I could be mistaken, though.

@lJoublanc
Copy link
Author

Hi Dave,

I'm not sure I agree with this one. IMO the natural zero for an order from, say, Amazon, is to not buy anything: 0 quantity, 0 price.

This is a perfect example - yes, mathematically this makes sense - but have you tried entering an order for zero items on amazon ..? Anyway not a big deal, I just thought it would be better suited to a Semigroup example.

The diagram looks ok to me, but it's probably down to interpretation. Is your issue the number of circles and stars in the lists on the left, middle, and right?

Yes exactly, this is what was confusing me. Now that you've explained it, I agree with you. As you said, it's a matter of interpretation.

Re: the future sequencing issue, It's probably my setup. I think I may also have had fs2 in my dependencies, pulling in a different version of cats altogether ...!

Re: tailRecM, the explanation you've added seems much clearer to me now, as you've elaborated on whether it needs to be tail recursive or not, and given both examples.

Can't wait to see the final version!

@davegurnell
Copy link
Contributor

davegurnell commented Sep 17, 2017 via email

@lJoublanc
Copy link
Author

May I close this off? I think most of the remaining unaddressed issues don't exist any more as the text has changed considerably.

@davegurnell
Copy link
Contributor

Absolutely. Sorry for my tardiness in replying. Sometime soon I'll reserve a few days to go through all these books and close off as many issues as I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants