5
\$\begingroup\$

I've started learning F# and functional programming in general, but the code I wrote doesn't seems to be really functional. Could you take a look and say how to make it more as it should be in functional programming?

type internal CsvReader(source : Stream) = 

    // input stream
    let stream = source

    // reader
    let mutable reader : StreamReader = new StreamReader(source)

    [<Literal>]
    let bufferSize = 1000

    // currently processed line
    let mutable buffer : char[] = Array.zeroCreate bufferSize

    // current position
    let mutable currentPosition : int = bufferSize

    // end of file
    let mutable EndOfFile : bool = false

    // loads new chunk of data from file to buffer
    let UpdateBuffer() : bool =
        match reader.ReadBlock(buffer, 0, buffer.Length) with
        | 0 -> false
        | _ -> true

    // initialization method
    let Initialize() =
        let pos = source.Seek(0L, SeekOrigin.Begin)
        if not (pos = 0L) then raise(Exception("sdf"))
        ()

    // read one char from stream
    let rec GetChar(eat : bool) : (bool * char) = 
        let position = currentPosition
        match position with
        | 1000 -> 
            match UpdateBuffer() with
            | false ->
                EndOfFile <- true
                (false, Char.MinValue)
            | true -> currentPosition <- 0; GetChar(eat)
        | _ -> 
            let item = System.Convert.ToInt32(buffer.[position])
            match item with
            | 0 -> (false, Char.MinValue)
            | _ ->
                match eat with
                | true ->
                    currentPosition <- currentPosition + 1;
                    (true, buffer.[position])
                | false -> 
                    (true, buffer.[position])

    // default constructor logic
    do
        if Utils.IsNull source then raise(ArgumentNullException("source"))
        if not source.CanRead then raise(ArgumentException("source"))
        if not source.CanSeek then raise(ArgumentException("source"))

        Initialize()

        ()

    // parameterless constructor
    // always throws ArgumentNullException
    private new() = CsvReader(null)

    // just for testing
    member x.PublicGetChar = GetChar

Right now the only think it does it stream content of Stream char by char by PublicGetChar method. I wrote UT to make sure it works:

[<TestClass>]
type CsvReaderTests() = 

    let GetStringStream(input : string) : Stream = 
        let stream = new MemoryStream()
        let writer = new StreamWriter(stream)
        ignore(writer.Write(input))
        writer.Flush()
        stream.Position <- 0L
        stream :> Stream

    [<TestMethod>]
    member this.TestMethod1 () = 
        let input = "test"
        let reader = new CsvReader(GetStringStream(input))
        let chars = seq { while fst(reader.PublicGetChar(false)) do yield snd(reader.PublicGetChar(true)) }
        let output = (new System.String(chars |> Array.ofSeq))
        input |> should equal output

Main concerns I have about the code:

  • usage of match/with
  • a lot of mutable stuff within CsvReader type
  • if I had to write it in C# it would look really similar :D

That's the very first F# code I've ever written, so be polite, please :)

\$\endgroup\$
  • \$\begingroup\$ What's the point of all this code, why won't you just use StreamReader.Read()? \$\endgroup\$ – svick Jan 25 '14 at 22:38
  • \$\begingroup\$ @svick Calling Read would require file access every time you call it. With this code you read from file in chunks, which should be faster. \$\endgroup\$ – MarcinJuraszek Jan 25 '14 at 22:40
  • 1
    \$\begingroup\$ No, it wouldn't. StreamReader already has an internal buffer. Anyway, whenever you do something that “should be faster”, you should really measure that it actually is faster. \$\endgroup\$ – svick Jan 25 '14 at 22:50
  • 1
    \$\begingroup\$ Yes, I would definitely measure that if it was production code. It's not :) I'm trying to learn f# and that's the only reason I wrote this code. \$\endgroup\$ – MarcinJuraszek Jan 25 '14 at 22:53
1
\$\begingroup\$

Your first F# code isn't too bad. I've made some small simplifications to streamline your code a bit and take advantage of some built-in F# functions:

type internal CsvReader(source : Stream) = 

    // input stream
    let stream = source

    // reader
    let mutable reader : StreamReader = new StreamReader(source)

    [<Literal>]
    let bufferSize = 1000

    // currently processed line
    let mutable buffer : char[] = Array.zeroCreate bufferSize

    // current position
    let mutable currentPosition = bufferSize

    // end of file
    let mutable EndOfFile = false

    // loads new chunk of data from file to buffer
    let UpdateBuffer() : bool =
        reader.ReadBlock(buffer, 0, buffer.Length) <> 0

    // initialization method
    let Initialize() =
        let pos = source.Seek(0L, SeekOrigin.Begin)
        if not (pos = 0L) then
            invalidOp "sdf"

    // read one char from stream
    let rec GetChar(eat : bool) : (bool * char) = 
        let position = currentPosition
        match position with
        | 1000 ->
            match UpdateBuffer() with
            | false ->
                EndOfFile <- true
                false, Char.MinValue
            | true ->
                currentPosition <- 0
                GetChar eat
        | _ -> 
            match int buffer.[position] with
            | 0 ->
                false, Char.MinValue
            | _ ->
                match eat with
                | true ->
                    currentPosition <- currentPosition + 1
                    true, buffer.[position]
                | false -> 
                    true, buffer.[position]

    // default constructor logic
    do
        if Utils.IsNull source then
            nullArg "source"
        if not source.CanRead then
            invalidArg "source" "The source stream cannot be read from."
        if not source.CanSeek then
            invalidArg "source" "The source stream does not support seeking."

        Initialize()

    // parameterless constructor
    // always throws ArgumentNullException
    private new() = CsvReader(null)

    // just for testing
    member x.PublicGetChar = GetChar

One thing you'd definitely want to add (if this were production code) -- this class would ideally implement the IDisposable interface so you can properly close and release the resources associated with the StreamReader you use internally.

\$\endgroup\$

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.