Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

In other languages, I prefer to arrange source files so that simpler and more widely useful concepts are introduced before implementation details, and I try where possible to make complex implementation details top-level functions so that I can separate them out and let the main function read like a high-level algorithm description.

OCaml obviously has an interface/source distinction, but even with a sparse well-documented interface like

type t
(** Blah blah blah *)

val important_operator : t -> t -> t
(** Blah blah blah *)

I've been tempted to do things like

let rec important_operator a b =
  let a', b' = foo a b in
  let a', b' = iterate_until_convergence (=)
                 (fun (a, b) -> bar (baz a b))
                 (a', b') in
  merge a', b'


(* Implementation details *)

and foo a b = ...

and bar x = ...

and baz a b = ...

and merge a b = ...

Is this poor style? Does it affect the ability of IDE-users to navigate source files?

share|improve this question
add comment

3 Answers

up vote 5 down vote accepted

It's not idiomatic OCaml indeed.

  1. "and" is an effective way to say "watch out for mutual recursion".
  2. OCaml encourages you to use nested functions instead! As The Structure of OCaml programs says: "Nested functions are, however, very useful and very heavily used in OCaml."

You would write this code as follows:

let important_operator a b =
  let foo a b = ... in
  let bar x = ... in
  let baz a b = ... in
  let merge a b = ... in
  let a', b' = foo a b in
  let a', b' = iterate_until_convergence (=)
                 (fun (a, b) -> bar (baz a b))
                 (a', b') in
  merge a', b'

Of course, you don't need to only use nested functions, you can also define bar or baz before defining important_operator. OCaml tends to only use functions that were defined "earlier", so using and for this is abusing the system.

share|improve this answer
add comment

The code is incorrect as written. The declaration of important_operator needs to start with "let rec" instead of just "let" in order to make the low-level functions visible when the main one is defined. And no, it's not idiomatic.

Nested functions are common, but there's not much point in them when they don't share data. You still have to write them in bottom-up order in the body of the outer function unless you use the "let rec" trick.

share|improve this answer
    
I updated the code to include the missing rec. Thanks for pointing that out. –  Mike Samuel Sep 18 '12 at 4:49
add comment

You would feel probably more at ease with a language like Haskell which follow that style. I think however that there is an camlp4 filter somewhere which provide a 'let .... where ...' syntactic construct allowing you to use that style. Still, while sometimes convenient, other comments above may have demonstrated how things are usually done in ocaml, and I think its usually efficient enough.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.