-
Notifications
You must be signed in to change notification settings - Fork 597
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
Make OCaml code samples closer to Haskell (chapters 1 to 4) #242
base: master
Are you sure you want to change the base?
Conversation
- Get rid of module declarations - Use only OCaml Stdlib - Rename composition operator to `(%)` due to operator precedence issues
@@ -1 +1,14 @@ | |||
let fact n = List.fold (List.range 1 n) ~init:1 ~f:( * ) | |||
(* OCaml doesn't ship with a lazy sequence range syntax nor a product |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I think I used Janestreet libs in a few places to reduce the snippet size and to move ahead quickly, but thanks for removing the dependency!
@@ -1,6 +1,6 @@ | |||
module type Monoid = sig | |||
type a | |||
module type MONOID = sig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it an OCaml convention to capitalize the module type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi yes I believe so, e.g. see http://www.mseri.me/typeclass-ocaml/
I will try to follow the ocaml conventions writing capitalized modules (e.g. Monoid) and fully capital signatures (e.g. MONOID).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the convention originates from SML, it's loosely followed in OCaml really (even stdlib doesn't follow it).
@@ -1 +1,4 @@ | |||
let pure x = x, "" | |||
let ( >=> ) m1 m2 = fun x -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The snippets are starting to diverge here but I hope they are in the right place in the book
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, the best way would probably be to run the LaTeX compilation and check the output, but from reading along the chapter I believe the sequence of the code samples now makes sense.
Even if we haven't reached a conclusion on whether we should write compilable snippets, I still think the changes in this PR, especially the ones that remove dependency on external libraries, are important. Thanks for fixing the order @yawaramin! I don't know when/how they got out of order |
Thanks @ArulselvanMadhavan I can work on more chapters as well, am enjoying reading through the book. |
Looks much cleaner and easier to read, indeed 👍 |
I agree. Looks good. Thanks @yawaramin |
Cheers guys, I appreciate the review 👍 |
Will this be merged? If so I will adapt the examples accordingly in my Reason branch, too. |
@@ -1,6 +1 @@ | |||
module type Polymorphic_Function_G = sig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was definitely confusing for me when I started going through the ocaml book (as an ocaml beginner when I saw these modules). 👍
Sorry for not attending to this. Are there more changes coming? Also, does this tie to the prelude discussion in #237? |
@hmemcpy hi, no problem. I've made some progress on Chapter 5 locally, if there is interest I can send more as I cover more chapters. It will be a good distraction for me from constantly obsessing over the news :-) |
I think a distraction would be welcomed for everybody! |
What is the current status of this MR? Is there a way to help? What about the other chapters? |
@hmemcpy would you prefer this PR to cover the whole book? Or separate PRs for a few chapters at a time? |
Hey all! So sorry for neglecting the PR! Would love to merge it. Can any of you verify there are no more changes? Unless you'd like to contribute the rest of the chapters as well? |
Hi, what's the status of this PR ? |
(G : Polymorphic_Function_G with type b = F.b) = | ||
struct | ||
(** OCaml doesn't have a compose operator. So, creating one. **) | ||
let ( >> ) g f x = g (f x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was confusing because earlier F#'s chevron operator was mentioned which is the same >>
operator but encodes forward composition. I have no idea why these examples were so excessively verbose! This isn't a good impression of the language's expressiveness
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so I've read earlier issues that explain why it was this verbose. interesting POVs.
| Seq.Cons (n, seq) -> product (result * n) seq | ||
let product = product 1 | ||
|
||
let fact n = product (range 1 n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just
let factorial n = Seq.fold_left ( * ) 1 (Seq.init n succ)
or
let product = Seq.fold_left ( * ) 1
let factorial n = Seq.init n succ |> product
or use List
instead of Seq
(at your choice, in case you prefer to be more backwards-compatible) ?
It doesn't really matter that it's "lazy" does it? At least the book doesn't seem to make a point out of it.
@@ -1,3 +1,2 @@ | |||
type void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type void = |
or just use the word void
directly, it's not like the haskell snippet has import Data.Void (Void, absurd)
@@ -1,6 +1,6 @@ | |||
module type Monoid = sig | |||
type a | |||
module type MONOID = sig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the convention originates from SML, it's loosely followed in OCaml really (even stdlib doesn't follow it).
fun s -> String.split s ~on:' ', "to_words " | ||
;; | ||
(* val up_case : string -> string writer | ||
Note: OCaml strings are raw bytestrings, not UTF-8 encoded. *) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The book never mentions UTF-8 encoding.. Maybe this makes sense in an OCaml tutorial to note, but why here?
@yawaramin I'm reading the book right now, and I'm happy to help if you're still working on this. If not, I can take it from here and revise the rest of the chapters. |
@hyphenrf hey that would be great, please feel free. I agree with all your comments btw. |
Hi all, What is the status of this PR ? Thanks |
hi @drupol, I haven't finished reading the book. I have notes written all over the OCaml impl, and once I've finished the book, I'll create a patch with all the edits and which closes this PR. |
Cool, glad to see it's still alive then. Looking forward for reviewing your pull request. Cheers! |
(%)
due to operator precedence issues