patternMinor
TCPListener server to discover clients on a network
Viewed 0 times
clientstcplistenerdiscoverservernetwork
Problem
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()
let serverLoop () =
let rec loop (pendingConnection:Task) = 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.
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()
let serverLoop () =
let rec loop (pendingConnection:Task) = 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.
Solution
I would consider altering
Basically, consumers of your class can actually modify the
In F#, I would probably do the following:
Or as Dan Lyons stated in comments:
This guarantees that you are exposing a non-modifiable copy of the list to the consumer of your class. You could optionally
Both of these have the same effect: the internal
I also noticed you use
member x.ActiveConnections for the reasons I mention on the answer here: Create database connection and run the insert, delete, update queries classBasically, 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 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.)Code Snippets
member x.ActiveConnections = activeConnections.ToArray()member x.ActiveConnections = activeConnections.AsReadOnly()member x.ActiveConnections = activeConnections.ToArray().AsEnumerable()Context
StackExchange Code Review Q#136791, answer score: 2
Revisions (0)
No revisions yet.