Tcp server messenger awaiter in f#

Question

Hi I have a simple tcp server, and that server receives messages with specific length so basically i need to wait for TcpClient to fill buffer with that amount of data, and it works.

But I am not sure if this should look like it looks as I am writing this in f# and still learning:

let private WaitForData (client : TcpClient, dataLength : int) : Async<bool>= 

    async {

        let mutable loopBreaker : bool = true
        let mutable result : bool = false

        while loopBreaker do
            let isConnectionAlive : bool = IsConnectionEstablished client && client <> null

            if isConnectionAlive && client.Available >= dataLength then 
                result <- true
                loopBreaker <- false


            if isConnectionAlive = false then 
                result <- false
                loopBreaker <- false                    

            Task.Delay(500) 
            |> Async.AwaitTask 
            |> ignore

        return result
    }

So as you see I use mutables to exit while loop and set result value, and have some if there.

Is this OK or maybe there is some more elegant way to write this

Answer 1

The first thing is when you use parenthesis it means that you pass a tuple to a function instead of single parameters. So you can ditch them.

The second thing is that bool supports pattern matching

The third thing is that you can use recursion instead of mutable variables. Note the usage of rec which enables tail call optimisation.

The code would look something like this.

let rec private WaitForData2 (client : TcpClient) (dataLength : int) (loopBreaker : bool) (result: bool) : Async<bool>= 
    async {
        match loopBreaker with
        | true -> 
            let isConnectionAlive : bool = IsConnectionEstablished client && client <> null
            match isConnectionAlive, client.Available >= dataLength with
            | true, true ->             
                Task.Delay(500) 
                |> Async.AwaitTask 
                |> ignore
                return! WaitForData2 client dataLength false true                
            | false, _ -> 
                Task.Delay(500) 
                |> Async.AwaitTask 
                |> ignore
                return! WaitForData2 client dataLength false false
            | _, _ -> 
                Task.Delay(500) 
                |> Async.AwaitTask 
                |> ignore
                return! WaitForData2 client dataLength true false
        | false -> return result     

Answer 2

let private WaitForData (client : TcpClient, dataLength : int) : Async<bool>=

let mutable loopBreaker : bool = true

let isConnectionAlive : bool = IsConnectionEstablished client && client <> null

You should only use explicit type declaration when the compiler demands it:

let private WaitForData (client : TcpClient) dataLength =

let mutable loopBreaker = true

let isConnectionAlive = IsConnectionEstablished client && client <> null

---

let isConnectionAlive : bool = IsConnectionEstablished client && client <> null

It seems rather risky to me that you use client in a function call before testing it for null. I would do it in reverse order:

let isConnectionAlive = client <> null && IsConnectionEstablished client

But why evaluate the client for null in the loop in the first place? You should return false immediately if it's null.

---

            Task.Delay(500) 
            |> Async.AwaitTask 
            |> ignore

I don't think this is actually waiting 500 ms as you may expect.

Instead you can do:

do! Async.Sleep 500

or

do! Task.Delay(500) |> Async.AwaitTask 

---

As Bohdan Stupak shows you can do the same in a recursive fashion like:

let private waitForData (client: TcpClient) dataLength = 
    let rec waiter () =
        async {
            if client = null then
                return false
            else 
                match isConnected client, client.Available >= dataLength with
                | true, true -> return true
                | true, false ->
                    do! Async.Sleep 500
                    return! waiter ()
                | false, dataFound -> return dataFound                            
        }
    waiter()