7
\$\begingroup\$

I am currently writing a program to sync files (music, photos, etc) from my PC to an Android device. In order to do this, I have 2 application: one that is running on my PC, and one that shall be running on the Android device. When the Android app will open, it will try to connect to a specified IP:port on the PC application, which has a server just listening for requests in order to show the clients in a WPF list style view. This server is written in F#.

Here is the server implementation I have come up with:

type TCPListenerServer(discoveryPort:int) =
    let server = new TcpListener (IPAddress.Loopback, discoveryPort)

    let activeConnections = new List<TcpClient>()

    let serverLoop () =
        let rec loop (pendingConnection:Task<TcpClient>) = async {            
            let newPendingConnection, client =
                match pendingConnection.Status with
                | TaskStatus.Created | TaskStatus.WaitingForActivation | TaskStatus.WaitingToRun | TaskStatus.Running  ->
                    (None, None)
                | TaskStatus.Faulted | TaskStatus.Canceled ->
                    raise (new System.NotImplementedException())
                | TaskStatus.RanToCompletion ->
                    let connectionTask = server.AcceptTcpClientAsync ()
                    (Some connectionTask, Some pendingConnection.Result)
                | _ -> 
                    raise (new System.NotImplementedException())

            // Add the new client to the list
            Option.iter (fun c -> activeConnections.Add c) client

            // Switch the new pending connection if there is one
            let connectionAttempt = defaultArg newPendingConnection pendingConnection

            Async.Sleep 1000 |> Async.RunSynchronously
            return! loop connectionAttempt
        }

        try
            server.Start ()

            let connectionTask = server.AcceptTcpClientAsync ()

            loop connectionTask
        finally
            server.Stop ()

    member x.Start () =
        serverLoop () |> Async.Start

    member x.Stop () =
        server.Stop ()

    member x.ActiveConnections =
        activeConnections

I wanted to know if my implementation was acceptable. I am unsure what is best: using a .NET List, or using a mutable variable that holds an F# list for activeConnections.

Also, usually I would keep the state of my object encapsulated in the serverLoop and just passing the updated list to each iteration, but doing this I cannot expose it so that I may have access to it from the outside (to bind on it in my WPF for example). Is there a better way to do this?

\$\endgroup\$
2
\$\begingroup\$

I would consider altering member x.ActiveConnections for the reasons I mention on the answer here: Create database connection and run the insert, delete, update queries class

Basically, consumers of your class can actually modify the ActiveConnections list (call .Remove, etc.), so you should find a way to eliminate that.

In F#, I would probably do the following:

member x.ActiveConnections = activeConnections.ToArray()

Or as Dan Lyons stated in comments:

member x.ActiveConnections = activeConnections.AsReadOnly()

This guarantees that you are exposing a non-modifiable copy of the list to the consumer of your class. You could optionally open System.Linq and use:

member x.ActiveConnections = activeConnections.ToArray().AsEnumerable()

Both of these have the same effect: the internal List<T> isn't able to be modified.


I also noticed you use x. as the prefix for all your member declarations, I'm not sure what the standard is (quick google pulled nothing) but I always try to use member this.Name for public-facing members, to remind myself that they're part of the same object. (F# doesn't seem to care much, but it still feels right.)

\$\endgroup\$
  • \$\begingroup\$ You could also use List<T>.AsReadOnly, which returns a ReadOnlyCollection<T> object. \$\endgroup\$ – Dan Lyons Sep 26 '16 at 18:20
  • \$\begingroup\$ @DanLyons Added that example as well. \$\endgroup\$ – Der Kommissar Sep 26 '16 at 18:24

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.