Can this reactive code be improved?

I have a view model that I'm migrating to RxUI. It exposes a collection of brokers along with a bool indicating whether there is a market (ie. at least one broker). In addition, it has a property for the currently selected broker (the user can choose a broker via a drop-down):

private IReactiveCollection brokers;
public IReactiveCollection Brokers
{
    get { return this.brokers; }
    private set { this.RaiseAndSetIfChanged(x => x.Brokers, ref this.brokers, value); }
}

private ObservableAsPropertyHelper isMarket;
public bool IsMarket
{
    get { return this.isMarket.Value; }
}

private SourceViewModel selectedBroker;
public SourceViewModel SelectedBroker
{
    get { return this.selectedBroker; }
    set { this.RaiseAndSetIfChanged(x => x.SelectedBroker, ref this.selectedBroker, value); }
}

I have these pieces hooked up and working, but I am not convinced I've done this in the most reactive-friendly fashion:

```
this.WhenAny(x => x.Volatilities, x => x.Value)
.StartWith((ItemObservableCollection)null)
.Subscribe(x =>
{
if (x == null)
{
// no volatilities, so definitely no market
this.Brokers = null;
this.SelectedBroker = null;
this.isMarket = Observable.Return(false).ToProperty(this, y => y.IsMarket);
}
else
{
// we have volatilities, so potentially we have a market

// TODO: is there a better means of doing this?
this.Brokers = x.CreateDerivedCollection(y => y.Source);

// TODO: is there a cleaner way to do this?
this.Brokers.Changed
.Select(y => true)
.StartWith(true)
.Subscribe(y =>
{
var volatility = x.FirstOrDefault();
this.SelectedBroker = volatility == null ? null : volatility.Source;

I currently create a new derived collection each time Volatilities changes

This was a good idea, but in this case I'd just recreate the brokers list (since it's not a straight volatilities.Select(x => ...), it's a SelectMany. One thing that's important though, is whether you mutate (i.e. add and remove) items from Volatilities, or you only replace the list (i.e. via an API call, set this.Volatilities to a new collection).

Pick one or the other, don't mix them or else your code will be complicated. Let's assume you're using ReactiveCollection and you're adding/removing to Volatilities.

Volatilities.CollectionCountChanged.StartWith(Volatilities.Count)
    .Select(_ => Volatilities.SelectMany(x => x.BrokerList))
    .ToProperty(this, x => x.Brokers);

Then, we can watch when someone changes Brokers - we'll do this in the constructor (along with everything else). Generally, if you're not calling RxUI methods in the constructor, you're Doing Something Wrong. You're wiring up on startup what will happen when various parts of your ViewModel change, those wires typically are constant:

this.WhenAny(x => x.Brokers, x => x.Value)
    .Subscribe(_ => SelectedBroker = Brokers.FirstOrDefault());

Now let's set up the IsMarket:

this.WhenAny(x => x.Brokers, x => x.Value)
    .Select(x => x.Count > 0)
    .ToProperty(this, x => x.IsMarket);

What We Can Learn™

The key point here though, is that IsMarket is directly related to Brokers and nothing else - we should describe that relationship without mentioning Volatilities, then separately describe the relationship between Volatilities and Brokers. We don't want to mix the two.

StackExchange Code Review Q#20900, answer score: 6