My First F# program

I just finish writing my first F# program. Functionality wise the code works the way I wanted, but not sure if the code is efficient. I would much appreciate if someone could review the code for me and point out the areas where the code can be improved.

Thanks Sudaly

open System
open System.IO
open System.IO.Pipes
open System.Text
open System.Collections.Generic
open System.Runtime.Serialization


[<DataContract>] 
type Quote = { 
    [<field: DataMember(Name="securityIdentifier") >] 
    RicCode:string
    [<field: DataMember(Name="madeOn") >] 
    MadeOn:DateTime
    [<field: DataMember(Name="closePrice") >] 
    Price:float 
    }

let m_cache = new Dictionary<string, Quote>() 

let ParseQuoteString (quoteString:string) = 
    let data = Encoding.Unicode.GetBytes(quoteString)
    let stream = new MemoryStream() 
    stream.Write(data, 0, data.Length); 
    stream.Position <- 0L 
    let ser = Json.DataContractJsonSerializer(typeof<Quote array>) 
    l开发者_如何学Goet results:Quote array = ser.ReadObject(stream) :?> Quote array
    results

let RefreshCache quoteList =
    m_cache.Clear()
    quoteList |> Array.iter(fun result->m_cache.Add(result.RicCode, result))


let EstablishConnection() =
    let pipeServer = new NamedPipeServerStream("testpipe", PipeDirection.InOut, 4)
    let mutable sr = null
    printfn "[F#] NamedPipeServerStream thread created, Wait for a client to connect"
    pipeServer.WaitForConnection()
    printfn "[F#] Client connected."
    try
        // Stream for the request. 
        sr <- new StreamReader(pipeServer)
    with
    | _ as e -> printfn "[F#]ERROR: %s" e.Message
    sr


while true do
    let sr = EstablishConnection()
    // Read request from the stream.
    printfn "[F#] Ready to Receive data"

    sr.ReadLine()  
    |>  ParseQuoteString  
    |>  RefreshCache

    printfn "[F#]Quot Size, %d" m_cache.Count
    let quot = m_cache.["MSFT.OQ"]
    printfn "[F#]RIC: %s" quot.RicCode
    printfn "[F#]MadeOn: %s" (String.Format("{0:T}",quot.MadeOn))
    printfn "[F#]Price: %f" quot.Price

---

In general, you should try using immutable data types and avoid imperative constructs such as global variables and imperative loops - although using them in F# is fine in many cases, they should be used only when there is a good reason for doing so. Here are a couple of examples where you could use functional approach:

---

First of all, to make the code more functional, you should avoid using global mutable cache. Instead, your RefreshCache function should return the data as the result (preferably using some functional data structure, such as F# Map type):

let PopulateCache quoteList = 
  quoteList 
  // Generate a sequence of tuples containing key and value 
  |> Seq.map (fun result -> result.RicCode, result)
  // Turn the sequence into an F# immutable map (replacement for hashtable)
  |> Map.ofSeq

The code that uses it would be changed like this:

let cache = 
  sr.ReadLine()   
  |>  ParseQuoteString   
  |>  PopulateCache

printfn "[F#]Quot Size, %d" m_cache.Count 
let quot = m_cache.["MSFT.OQ"] 
// The rest of the sample stays the same

---

In the EstablishConnection function, you definitely don't need to declare a mutable variable sr, because in case of an exception, the function will return null. I would instead use option type to make sure that this case is handled:

let EstablishConnection() = 
    let pipeServer = 
      new NamedPipeServerStream("testpipe", PipeDirection.InOut, 4) 
    printfn "[F#] NamedPipeServerStream thread created..." 
    pipeServer.WaitForConnection() 
    printfn "[F#] Client connected." 
    try // Wrap the result in 'Some' to denote success
        Some(new StreamReader(pipeServer))
    with e -> 
        printfn "[F#]ERROR: %s" e.Message 
        // Return 'None' to denote a failure
        None 

The main loop can be written using a recursive function that stops when EstablishConnection fails:

let rec loop() =
  match EstablishConnection() with
  | Some(conn) ->
      printfn "[F#] Ready to Receive data"
      // rest of the code
      loop() // continue looping
  | _ -> () // Quit

---

Just a couple thoughts...

You probably want a 'use' rather than a 'let' in a few places, as I think some of the objects in the program are IDisposable.

You may consider wrapping the EstablishConnection method and the final while loop in async blocks (and make other minor changes), so that e.g. you can wait asynchronously for connections without blocking a thread.

---

At first glance it is written in imperative style rather than functional style, which does make sense given that most of the program involves side effects (i.e. I/O). Line for line, it almost looks like a C# program.

Given the amount of I/O that is taking place, I don't know that there is much you can do to this particular program to make it more of a functional style of coding.