2
votes

I wrote the following function while following Real World OCaml, which uses the Core library.

open Core.Core_list
open Core.Option
open Core.Std
open Re2

let getMaxFilename target = 
    let Ok pat = Regex.create "^.*(..)\\.txt$" in
    Sys.ls_dir target |> 
    List.map    ~f:(Regex.find_submatches pat) |>
    List.filter ~f:is_ok |>
    List.map    ~f:(fun x -> ok_exn x |> Array.to_list |> (Fn.flip nth_exn) 1 |> fun x -> value_exn x) |>
    List.reduce ~f:max

It looks messy to me since I have a lot of "opens" at the top and I have to name List, Array, Sys, Fn, and the other modules names in all the functions that I use. This is the "right" way to write OCaml? Is there a standard style that dispenses with these?

1
You don't need 1st two lines there: Core.Core_list is alias to Core.Std.List and Core.Option to Core.Std.Option. So, I usually use open Core , open Core.Std and have fun - Kakadu
@Kakadu why not post it as answer? - lukstafi
@Kakadu I tried open Core and open Core.Std, but I do not get access to functions like flip or nth. Sounds like I still need to name many of the modules. Is this the right way to write Ocaml code? - Ana
@Ana, I like to write List.nth verbosely to be sure from what module the function is. Of course, we should find a way to keep balance, between readability and verbosity. - Kakadu
@lukstafi, My post seems to be more like small tip than a full answer. - Kakadu

1 Answers

0
votes

I'm not sure this is the best way to do this, but here's a fairly straight-ahead stylistic cleanup, without really doing anything material.

open Core.Std
module Regex = Re2.Regex

let get_max_filename target = 
  let pat = Regex.create_exn "^.*(..)\\.txt$" in
  Sys.ls_dir target
  |> List.map ~f:(Regex.find_submatches pat)
  |> List.filter_map ~f:Result.ok
  |> List.filter_map ~f:(fun x -> x.(1))
  |> List.reduce ~f:max

Generally speaking, heavy use of open is frowned upon.

The following might be yet clearer and easier to follow.

let get_max_filename target = 
  let pat = Regex.create_exn "^.*(..)\\.txt$" in
  Sys.ls_dir target
  |> List.filter_map ~f:(fun entry ->
      match Regex.find_submatches pat entry with
      | Error _ -> None
      | Ok ar -> ar.(1))
  |> List.reduce ~f:max