patternMinor
Good usage of MailboxProcessor/Agent in class rather than mutable field?
Viewed 0 times
mailboxprocessorfieldthanagentratherusagegoodclassmutable
Problem
I've written a simple Simple Service Discovery Protocol (SSDP) module. This module contains a class,
The usage looks like this:
It is intended that you hang on to the
The question: What would be the elegant/idiomatic options to handle the changing of the IP and hence
What I have done so far, is run an
It looks like this:
```
let udpClientListeningController =
Agent.Start(fun inbox ->
let rec loop (runningClients:UdpClient seq) = async {
let! msg = inbox.Receive()
let closeRunningClients () =
runningClients |> Seq.iter (fun client -> client.ProtectedClose())
match
Session that you create and then use to issue commands and subscribe to interesting events. I include the relevant code snippets below, but you can also view the entire SSDP module gist.The usage looks like this:
//Fire up a session.
let ssdpSession = Ssdp.Session.Start(1985)
//Startlistening
ssdpSession.MessageReceived.Add(fun msg -> printfn "%s" msg)
//Do an MSearch.
let search = { MSearch.SearchTarget = All; DeviceResponseDelay = OneSecond; }
ssdpSession.BroadcastMSearch(search)It is intended that you hang on to the
Session instance for as long as you need it. However, the Session is listening on one or more IP addresses for the lifetime of your usage, and these IP addresses may change (for example, you renew an IP DHCP lease). The Session class uses System.Net.NetworkInformation.NetworkChange.NetworkAddressChanged to detect this.The question: What would be the elegant/idiomatic options to handle the changing of the IP and hence
UdpClient listener set? I am trying to avoid mutable fields, but I wonder if I'm going too far with my approach below (just tell me, am I an astronaut!?). Are there elegant/idiomatic approaches for this?What I have done so far, is run an
MailboxProcessor (Agent) that fires up the UdpClient listeners and then waits for a message to fire up a new set (or tear them all down for good). The current set is passed to the recursive function running in the agent, so we always have it to hand when a change is requested or it is time to kill things off.It looks like this:
```
let udpClientListeningController =
Agent.Start(fun inbox ->
let rec loop (runningClients:UdpClient seq) = async {
let! msg = inbox.Receive()
let closeRunningClients () =
runningClients |> Seq.iter (fun client -> client.ProtectedClose())
match
Solution
Your code looks pretty good to me - I would only change some minor details.
-
I'd replace
-
I'd probably write the creation of clients differently - I think the following is easier to understand:
-
You're calling
-
I'd replace
UdpClient seq with UdpClient list, because lazy sequences can be subtle. In your code, this doesn't seem to be a problem, but they can cause unexpected GC issues (by capturing a reference) and tricky performance behavior (by re-evaluating things).-
I'd probably write the creation of clients differently - I think the following is easier to understand:
let uniAndMulticastUdpClients =
[ for ip in ipAddresses do
yield createUnicastClient(ip, port)
yield createMulticastClient(ip) ]-
You're calling
closeRunningClients in both branches, so you can extract it and move it before the match construct (and, in fact, you can just use the Seq.iter call without defining a function for it).Code Snippets
let uniAndMulticastUdpClients =
[ for ip in ipAddresses do
yield createUnicastClient(ip, port)
yield createMulticastClient(ip) ]Context
StackExchange Code Review Q#60702, answer score: 2
Revisions (0)
No revisions yet.