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 *)
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.
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.
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.
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.
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).
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
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.
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.
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.