Read a file line by line and print line numbers

I’ve written a program to read a file line by line and print the line numbers.
My question, what do you think of it ? Good/bad/ugly.

let print_file channel =
    let t = ref 0 in
    let rec pf channel =
        let ilc = input_line channel in 
        match ilc with
        | line ->
            incr t ;
            Printf.printf "%d:" !t ;
            print_endline line ;
            pf channel
        | exception End_of_file -> 
            () 
        in
    pf channel 
    in
let fn = "file.txt" in
let ic = open_in fn in
    try 
        print_file ic ;
        close_in ic                 (* close the input channel *) 
    with e ->                       (* some unexpected exception occurs *)
        close_in_noerr ic ;         (* emergency closing *)
        raise e ;;                  (* exit with error: files are closed but channels are not flushed *)

A few things that could be improved in my eyes:

  • You could use the format string "%d: %s\n"
  • You could avoid the mutable reference for line number by simply having it as an argument to pf recursive function and increment it on each recursive call.
  • The ressource handling looks right and exception safe but you can make it more trivial by using Fun.protect.
4 Likes

I have two remarks:

  • First, with “%d” only lines with the same number of digits will be aligned. You could use “%-4d” to have left-aligned space-padded 4-digit numbers to avoid this issue
  • Second, extracting the line one at a time is essentially creating an implicit sequence of lines. You could define a sequence explicitly with:
let next_line_with_number (n,ch) = match input_line ch with
  | x -> Some ((n,x),(n+1,ch))
  | exception End_of_file -> None
let numbered_lines ch = Seq.unfold next_line_with_number (1,ch)

with this split, you can separate the reading and printing code and use standard library (or Fmt ) functions for printing sequences:

let newline ppf () = Format.fprintf ppf "@,"
let print_with_number ppf (n,l) = Format.fprintf ppf "%-4d %s" n l
let print ppf numbered_lines =
  Format.pp_print_seq ~pp_sep:newline print_with_number ppf numbered_lines
        let ilc = input_line channel in 
        match ilc with
        ...
        | exception End_of_file ->

I don’t think that this is doing what you expect. match e with exception catches exceptions that happen when e is evaluated. In that case, it’s a plain variable so it can not raise. If input_line raises, the exception is going to bubble up until the try ... with at the end.

Functionally I don’t think there’s much difference in that particular case but it’s always going through the “emergency closing” branch at the moment.

3 Likes

Can the raising of the exception at the end be prevented by changing the code a bit ?
Normally you use exceptions for “real errors”

If you want the first exception handler to catch End_of_file, you’ll need to do match input_line channel with instead, with no intermediate ilc binding.

In the upcoming OCaml 4.14 you will be able to use the brand new In_channel.input_line which returns None on end of file rather than raising an exception.

4 Likes

Exceptions and errors are different things, and in functional programming it’s actually pretty common to use exceptions for exceptional control flow that’s not due to errors. For example, it’s often convenient to implement search by raising an exception (not an error!) when you find a solution.

1 Like

Incidentally, starting in 4.14 you will be able to do:

let () =
  let s = In_channel.with_open_text "file.txt" In_channel.input_all in
  List.iteri (Printf.printf "%d: %s\n") (String.split_on_char '\n' s)

Cheers,
Nicolas

3 Likes

Note that this is not equivalent to the code @devosalain submitted for review (no line translation, no final empty end line suppression) and needs to input all the data in memory (which you may not want depending on your use case).

2 Likes

I would check out Stream.from, it could really simplify your code.

https://ocaml.org/releases/4.02/htmlman/libref/Stream.html

Something like:

let () =
    let filename = print_string "Enter filename: "; read_line() in
    let ifile = open_in filename in
    let str =
        Stream.from
        (
            fun x ->
                try
                    Some( (string_of_int (x + 1))^ ": " ^(input_line ifile))
                with
                Stream.Failure
                | _ -> close_in ifile; None
            ) in
    Stream.iter print_endline str

Note that streams are deprecated from the Stdlib – but you can still use them via the camlp-streams package.

That must be recent because the online docs don’t mention that.

opam install line_oriented

open Printf
module LO = Line_oriented

LO.iteri "file.txt" (printf "%d:%s\n");
2 Likes

That deprectation of Streams from Stdlin has been withdrawn, because of some compilation problems, see previous reference to the @dbuenzli post. But it will be fixed and implemented. So I guess new code should probably not use Stream or depend on camlp-streams package.

2 Likes

wait, wut? So in future, there will simply be on Stream package? If that’s the case, I need to make a copy for camlp5 – could I get an official answer on this?

The Stream module will be deprecated but still present in the standard library in 4.14, thus every package depending on the Stream module will still work (except for dune hardline stance on deprecation alert in development build). But yes, if possible, it could make sense to try to move camlp5 to the package camlp-streams and report any troubles.

1 Like

Please follow and read the link that dbuenzli added to his post.
You can see that Stream will be packed in another package, just because of camlp5.

1 Like

Oh, ok. I interpreted the highlighted text as saying “don’t do either of these things” (“not (A or B)”) instead of do either of these things (“(not A) or B”).

Sorry for the confusion. Need to upgrade my English parser.

Sorry, English isn’t my first language, and I see now how you made the interpretation.
I will try to be a bit clearer in the future. Can’t promice though. :slight_smile:

1 Like