8
\$\begingroup\$

In the process of learning F#, I wrote this code that reads lines of input from the console and stores them in a list.

As far as I can tell the code works correctly, but since I'm new to functional programming, I'm not sure if it is written as well as it could have been.

  1. Should I have read characters iteratively instead of recursively?
  2. Is there a better alternative to using a StringBuilder object?
  3. Does my code follow proper functional style?

open System

/// Appends a character to the string builder and returns the new builder.
/// Characters 10 and 13 (newline and carriage return) are ignored.
/// Returns the updated StringBuilder.
let appendChar char (builder : Text.StringBuilder) =
    match char with
    | 10 | 13 -> builder
    | n ->
        let c = Convert.ToChar(n)
        builder.Append(c)

/// Finishes building a string by clearing the StringBuilder
/// and appending the string to the end of the list of strings.
/// Empty strings are ignored.
/// Returns a tuple containing the lines and the new StringBuilder.
let finishString lines (builder : Text.StringBuilder) = 
    let s = builder.ToString()
    let l = builder.Length
    let newBuilder = builder.Clear()
    match l with
    | 0 -> (lines, newBuilder)
    | _ -> (lines @ [s], newBuilder)

/// Handles a character by appending it to the builder and taking an appropriate action.
/// If char is a newline, finish the string.
/// Returns a tuple containing lines and the new builder.
let handleChar char lines builder =
    let newBuilder = appendChar char builder
    match char with
    | 10 -> finishString lines newBuilder
    | c -> (lines, newBuilder)

/// Gets all the lines from standard input until end of input (Ctrl-Z).
/// Empty lines are ignored.
/// Returns a list of strings read.
let rec getLines lines builder =
    match Console.Read() with
    | -1 -> lines
    | c ->
        let tuple = handleChar c lines builder
        let newLines = fst tuple
        let newbuilder = snd tuple
        getLines newLines newbuilder

[<EntryPoint>]
let main argv = 
    Text.StringBuilder()
    |> getLines []
    |> ... and so on
\$\endgroup\$
8
\$\begingroup\$

Some small comments

    let tuple = handleChar c lines builder
    let newLines = fst tuple
    let newbuilder = snd tuple
    getLines newLines newbuilder

can become

    let newlines,newbuilder = handleChar c lines builder
    getLines newLines newbuilder

although, if you aren't using getLines anywhere else, there is a good argument for making it

handleChar c lines builder |> getLines

by makeing getLines take its argument in tupled form.

The magic number 10 appears a few times, so I would add a literal like

[<Literal>]
let newline = 10

and then you can pattern match against it.

Also,

Lines @ [s]

is bad as @ is very slow. You are better to use

s::Lines

and then reverse the list at the end.

Your format for reading the characters recursively should be fine as it will get optimised as a tail-call.

Of course, your entire program could be replaced by:

let rec procinput lines= 
    match Console.ReadLine() with
    | null -> List.rev lines
    | "" -> procinput lines
    | s -> procinput (s::lines)
\$\endgroup\$
  • \$\begingroup\$ Wouldn't your version of procinput return the lines in reverse order? I think it should be null -> List.rev lines. \$\endgroup\$ – svick Jan 19 '14 at 22:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.