I have a review request about this part of the code:

let private startReading (client:TcpClient) (bus:IBus) = 
    //TODO check if that can be changed to rec parameter
    let frameNumber = ref 0
    let rec StartReading (client:TcpClient) =
            match client with
            | null -> async { return 0 } |> Async.StartAsTask |> ignore
            | _ ->
                match (client.IsConnectionEstablished()) with
                | true ->
                    incr frameNumber
                    let! messange = MessagingHandler.ReadMessage client

                    match(messange) with
                        | Some data -> 
                            let frameNumber = uint32 frameNumber.Value
                            logger.Info(sprintf "Received message with Frame Number: %i" frameNumber) 

                        | None -> 
                            _logger.Info(sprintf "Client disconnected : %O" client.Client.RemoteEndPoint)
                            return! DisconnectClient client None                                

                    return! StartReading client
                | false ->
                    _logger.Info(sprintf "Client disconnected : %O" client.Client.RemoteEndPoint)
                    return! DisconnectClient client None

    StartReading client |> Async.StartAsTask

It is simple reading message procedure from tcp client in f#, but I feel this can be written more gracefully, especially todo part with frame number incrementation using ref in f# feels wrong.

Any suggestions?


1 Answer 1


Why do you make this call Async.StartAsTask instead of just return the Async<T> to let the client handle that as needed?

let rec StartReading (client:TcpClient) =

I would call it reader and then give it the signature of:

let rec reader frameNo (client:TcpClient) =

in order to get rid of reffor frameNumber.

You can then initially call it:

reader 0 client |> Async.StartAsTask

and recursively:

return! reader (frameNo + 1) client

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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