Request for style critique of a simple program

Learning
1

Hello everyone. This is (something like) my first OCaml program. I look forward to your comments towards improving style.

(* Ask user for coordinates of two points.
 * Echo the inputs.
 * Print the Euclidean distance between the two points. *)

let askf prompt =
  print_string (prompt ^ ": ");
  read_float ()
  
let showf label value =
  print_string (label ^ " = " ^ string_of_float value ^ "\n")

type point = { name : string; x : float; y : float; z : float }

let ask_point name =
  print_endline ("For the " ^ name);
  let x = askf "X" in
  let y = askf "Y" in
  let z = askf "Z" in
  { name = name; x = x; y = y; z = z }

let show_point p =
  print_endline (p.name);
  showf "X" p.x;
  showf "Y" p.y;
  showf "Z" p.z;
  ()

let distance p1 p2 =
  let square x = x *. x in
  sqrt ((square (p1.x -. p2.x)) +.
        (square (p1.y -. p2.y)) +.
        (square (p1.z -. p2.z)))
  
let main =
  let p1 = ask_point "Point 1" in
  let p2 = ask_point "Point 2" in
  
  print_newline ();

  show_point p1;
  show_point p2;

  print_newline ();
  
  showf ("Distance between the points") (distance p1 p2);
  
  exit 0

let () = main
4 Likes
2

You can use a module fort the point. module Point, type t = { … }, then other point-related functions. :slight_smile: So you have show instead of show_point.

4 Likes
3

Use punning to make this a little cleaner:
{name; x; y; z}

Use printf to reduce allocations:
Printf.printf "%s = %f\n" label value

5 Likes
4

I think the solution is fine for the problem at hand. If you want to think ahead, you might want to define point differently. Because if you create intermediate points you now need to name each, which is inconvenient. So I would decouple names from the type representing points.

1 Like
5

A couple of thoughts.

OCaml is eagerly evaluated, so when you do something like:

let main = print "Hi"
let () = main

The Hi is printed as soon as the first line is evaluated, therefore the second line is redundant. To delay the evaluation of the first line, you need to make it a function:

let main () = print "Hi"
let () = main ()

One more thing, in this definition,

let distance p1 p2 =
  let square x = x *. x in
  ...

Technically there’s nothing wrong with it, it will work as intended. It’s just slightly inefficient because in theory, it recreates a new square function every time distance is called. Now practically there may be nothing wrong with this–different compiler versions might just optimize away the definition. But you don’t need to rely on that–you can lift out the square function yourself:

let square x = x *. x

let distance p1 p2 =
  ...

Now, square is defined once, in the same module, no matter how many times distance is called.

3 Likes
6

You can also re-use the eager evaluation to have square created only once while being locally scoped. It’s pattern used in many OCaml projects:

let distance =
  let square x = x *. x in
  fun p1 p2 -> ...
6 Likes
7

I’m curious, does the vanilla or flamda compiler always remove these kind of redundant closures? I know that the compiler doesn’t do too many optimisations, but this seems like an obvious one that wouldn’t change the runtime behaviour of a function in too much of an unexpected way. If not, I guess I have quite a few functions to rewrite I guess. I like to nest functions for the purpose of code clarity and scoping - but explicitly shifting the function parameters to the end of the definition I find hurts readability.

1 Like
8

This came up before and @vlaviron said that it depends on whether the local function takes arguments from its context (a closure). So writing the local function as though it was external should avoid additional allocation.

5 Likes
9

And in this particular case the function is inlined.

6 Likes
10

Wow, thanks for the many helpful comments!

Incidentally, I noticed in one of the replies that when quoting code here, if I use Markdown’s ~~~~ instead of <pre></pre> tags, I get the syntax highlighting. :grin:

let foo =
11

Since I didn’t see any mention of it, I guess there is no better way of grabbing three consecutive inputs than with repeated let expressions?

  let x = askf "X" in
  let y = askf "Y" in
  let z = askf "Z" in
12

I wasn’t aware of the ~ syntax, I usually use the triple-backtick ‘fenced code block’ to format code with syntax highlighting: Extended Syntax | Markdown Guide

… I guess there is no better way of grabbing three consecutive inputs than with repeated let expressions?

There is Scanf, which works similarly to C’s scanf function and allows formatted data entry from the input. E.g.:

(* test.ml *)

type point = { x : float; y : float; z : float }

let ask_point name =
  Printf.printf "Input point %s x,y,z: %!" name;
  Scanf.scanf "%f,%f,%f" begin fun x y z -> { x; y; z } end

let () =
  let { x; y; z } = ask_point "origin" in
  Printf.printf "x=%f,y=%f,z=%f\n%!" x y z

You can run it like so:

$ ocaml test.ml
Input point origin x,y,z: 1,2,3
x=1.000000,y=2.000000,z=3.000000
3 Likes
13

Oh, that’s nice! I thought scanf would require a ref, like in C, or – I think I saw something about a function argument but I didn’t get it. Now I see how it works. Cool!

3 Likes
14

This is such a lovely thread. What a great way to learn!

3 Likes
15

What about List.map with destructing? let [x, y, z] = List.map (fun → scanf …) [“Enter x”, “Enter y”, “Enter z”]?

16

Destructuring a list directly is not recommended as it doesn’t handle lists of other lengths. The compiler warns about that and dune turns warnings into errors.

17

Weird. Feels like a perfectly reasonable case to me. :blush:

18

You can think of this code snippet as being equivalent to a match statement that raises:

let [x, y, z] = List.map (fun → scanf …) [“Enter x”, “Enter y”, “Enter z”] in
BODY

is equivalent to

match List.map (fun → scanf …) [“Enter x”, “Enter y”, “Enter z”] with
| [x; y; z] -> BODY
| _ -> raise (Match_failure (...))

But the latter is explicit about what happens in the error case; in the former, it is not obvious that the let-binding itsel fcan raise an exception.

2 Likes