Re: reverse a list. is this a good solution

Let's unpack it a bit. Going back to your original function:

let rec test l1 l2 =
match l1 with
| [] -> l2
| h :: t -> test t (h::l2)
;;

this works, but it has the problem that l2 should always be an empty list
for the initial call. One way to solve it is to add that to the function
documentation:

(* NOTE: Always call this with l2 = [] *)
let rec test l1 l2 =
match l1 with
| [] -> l2
| h :: t -> test t (h::l2)
;;

but that is pretty fragile; the user could always ignore that. A better way
is to provide a second, wrapper function that takes only one argument, l1,
and then calls your original function with l1 and []:

let rec test l1 l2 =
match l1 with
| [] -> l2
| h :: t -> test t (h::l2)
;;

let reverse_list l1 =
  test l1 []

This is a lot better already - you cannot go wrong if you call
reverse_list. But there is still the issue that test is exposed, and could
inadvertently be called. If you think about it, you want a function to
reverse a list, and therefore it should only take one argument, l1. So
where did l2 come from? The issue is that if we are building up a list via
recursive calls to the same function, we need a second argument, the
accumulator, to store the in-progress intermediate results. But this is not
really part of your function interface; it is an implementation detail that
has been exposed to the user. What you *want* is for reverse_list to be
your public interface, and test to be hidden entirely. The common way to do
that is to define test entirely within the body of reverse_list:

let reverse_list l1 =
  (* here we define the 'test' function *)
  let rec test l1 l2 =
    match l1 with
    | [] -> l2
    | h :: t -> test t (h::l2)
  in
  (* here 'test' has been defined and is available *)
  test l1 []

(* here 'reverse_list' has been defined and is available but
   'test' is no longer in scope
 *)

now the user can only call reverse_list with a single list argument, l1,
which is exactly what you want.

martin