Update OCaml snippets #346
Conversation
|
Thank you so much for this MASSIVE effort! Let's wait for a few LGTMs. And thank you again! |
|
Hi, I'm doing a review. Will get back soon. |
| compose (fmap f) safe_head = compose safe_head (fmap f) | ||
| (* Given a Functor implementation for Option and List, | ||
| the following equality should hold: *) | ||
| OptionFunctor.fmap f % safe_head = safe_head % ListFunctor.fmap f |
There was a problem hiding this comment.
I feel like using these specific micro-modules is not really idiomatic for OCaml. Imho it should be more like:
Option.map f % safe_head = safe_head % List.map fSpecifically the name fmap is a Haskellism because they need to distinguish it from map which is specifically for lists, IIRC. In OCaml we would use the module name for that, so fmap doesn't really make sense.
| (* Starting with empty list *) | ||
| let fmap f (safe_head []) = fmap f None = None | ||
| (* As a reminder, this is not actual code *) | ||
| OptionFunctor.fmap f (safe_head []) |
There was a problem hiding this comment.
Same comment as above for the use of the OptionFunctor.fmap et al.
| @@ -1,2 +1,4 @@ | |||
| (* Starting with empty list *) | |||
| let fmap f (safe_head []) = fmap f None = None | |||
| (* As a reminder, this is not actual code *) | |||
There was a problem hiding this comment.
To clarify, I would say something like: 'As a reminder, this is not actual OCaml code, it is a demonstration of equational reasoning'.
There was a problem hiding this comment.
Honestly, there are a lot of snippets that aren't really code (just basically equations) and the Haskell version would suffice in those cases. Maybe these could be removed instead of duplicating the equational reasoning?
| | [] -> [] | ||
| | x :: xs -> f x :: fmap f xs | ||
| ;; | ||
| module ListFunctor : Functor with type 'a t = 'a list = struct |
There was a problem hiding this comment.
Imho, OCaml will derive this type without any annotation just like in Haskell. We could just show the implementation ie:
(* list.ml *)
type 'a t = 'a list = ...
let rec map = function ...And this would be enough to fulfill a Functor signature. We could even mention that this is provided in the standard library.
There was a problem hiding this comment.
The type annotation was just so the reader can be sure that the module ListFunctor implements the functor signature which won't be very usable if the type 'a t = 'a list is hidden by the : Functor annotation.
If we made the previous snippets use the List.map/Option.map it could be better, removing the need for these example modules.
| | Some x -> Some (f x) | ||
| end | ||
|
|
||
| (* As a reminder, note that in the OCaml Stdlib, the List |
There was a problem hiding this comment.
Ah, I see you mentioned this; I would recommend just showing the implementations as they would be in the standard library instead of trying to implement 'instances' as micro modules and the fmap name, both of which are Haskellisms.
| @@ -1,3 +1,3 @@ | |||
| module Chapter2_Bottom : Chapter2_DeclareFunction = struct | |||
| let f : bool -> bool = fun _ -> failwith "Not implemented" | |||
There was a problem hiding this comment.
Actually, we can't delete the fun _ ->, because OCaml will eagerly evaluate the failwith and immediately raise the exception as soon as the module is loaded.
There was a problem hiding this comment.
Actually, after revisiting this part of the book, I feel like this isn't technically wrong, since the point of the example is to show that this typechecks. Maybe I could add a comment noting the behaviour you mentioned. What do you think?
| @@ -1,8 +1,4 @@ | |||
| module Chapter5_Product_Example : | |||
| Chapter5_Product | |||
| with type a = int | |||
There was a problem hiding this comment.
The with operator isn't adding much to this chapter example, I think. It's not like in the case of the Functor, Monad, Monoid, etc. signatures where you'd like to be able to call your implementation outside of the module itself, for example. Here, we're just constraining the types inside this module for the sake of the book's example.
| @@ -1 +1,2 @@ | |||
| let stmt = "This statement is", false | |||
| (* You cannot do that in OCaml *) | |||
There was a problem hiding this comment.
This wording is confusing because we can obviously use comma in infix position to create tuples. Maybe we should say something like 'Comma cannot be used as an operator in OCaml, but we can define a custom operator which creates a pair', eg
let ( *. ) x y = x, y
let stmt = ( *. ) "This statement is" falseThere was a problem hiding this comment.
Indeed, how about a comment something like:
(* The OCaml compiler parses tuples directly using comma as the separator,
thus ( , ) is not actually an operator like in Haskell. Instead you can define a
custom infix operator that behaves like ( , ), e.g.: *)
As discussed in #345, I've updated most of the OCaml snippets to be simpler, not depend on anything besides the standard library, and also fixed mistakes. I've also changed some formatting choices for consistency throughout the book.
I built the book locally and it seemed fine, hopefully everything's alright.