SortAndBind fixes and improvements

[RolandPheasant]

• Enable opt-in to ResetOnFirstTimeLoad to ensure backwards compatibility with the old Sort(...).Bind(...) behaviour.

To opt-in system-wide after upgrading from v8 to v9 (and migrating to SortAndBind) like this:

DynamicDataOptions.SortAndBind = DynamicDataOptions.SortAndBind with { ResetOnFirstTimeLoad = true };

• Specialised handing for BindingList when used with SortAndBind, to ensure reset is respected.

• Specify SortAndBind scheduler at a system with level.

DynamicDataOptions.SortAndBind = DynamicDataOptions.SortAndBind with { Scheduler= myMainThreadScheduler };

With this ObserveOn(myMainThreadScheduler) will be a thing of the past.

Fixes #929

[dwcullop]

Looks reasonable to me.

[JakenVeina]

This is insufficient. Refer to #932, the logical query this needs to serve as a replacement for is...

.Sort(...)
.ObserveOn(...)
.Bind(...)

I.E. the scheduling needs to be applied to both input streams, source and comparer. Otherwise, notifications coming in on comparer can trigger the ObservableCollection be manipulated off of the main thread.

Another .ObserveOn() call applied to comparer should be sufficient here, since there is no change stream "between" the sorting and binding steps anymore.

[RolandPheasant]

Fair call, I'll fix that.

Also I was wondering whether to add scheduler overloads to SortAndBind, or instead to instruct the use of

DynamicDataOptions.SortAndBind = DynamicDataOptions.SortAndBind with
{
    Scheduler = myMainThreadScheduler
};

which is neat but not obvious.

Or should I do both? Thoughts?

EDIT:

Or better yet set the scheduler like RxUI:

DynamicDataOptions.MainThreadScheduler= myMainThreadScheduler;

[RolandPheasant]

I've applied the scheduler to the comparer changed observable, and moved the UI thread scheduler option to DynamicDataOptions.MainThreadScheduler.

So the one remaining question is whether it's worth adding 12 new overloads to pass an optional comparer to SortAndBind or to rely on the global main thread scheduler only?

[JakenVeina]

I'd say both. There's value in being able to specify on a per-call basis, and in being able to set it globally and not have to worry about it.

Not sure about whether a global-level scheduler should be universal, or per-operator. Again, I think there's value in both, but in this case, I think it's a little cumbersome to HAVE both.

Rather than MainThreadScheduler, I would suggest BindScheduler or BindingScheduler because that clarifies how it's actually being USED if you set it. It would only be pulled for binding operations.

[RolandPheasant]

Renamed to BindingScheduler + added scheduler overloads for new generation of binding operators.

[dwcullop]

Also I was wondering whether to add scheduler overloads to SortAndBind, or instead to instruct the use of

I suggest both: Giving the ability to provide all the options for their special cases AND providing a helper method that facilitates the most common use case(s).

[dwcullop]

I think that BindingScheduler should go in SortAndBindOptions instead of the top level DynamicDataOptions. It seems confusing to have ADDITIONAL "Binding" options at the top level when there is already a section for Binding specific options.

If it applies to both SortAndBindOptions and BindingOptions, then it can be in both, allowing the ultimate in flexibility for consumers.

You could also do both: Add a Scheduler property to SortAndBindOptions and a DefaultScheduler property on DynamicDataOptions and then coalesce the values.

scheduler ??= localSortAndBindOptions.Scheduler
    ?? DynamicDataOptions.SortAndBind.Scheduler
    ?? DynamicDataOptions.DefaultScheduler
    ?? Scheduler.Default;

[dwcullop]

If you do that way, then you don't need a separate IScheduler parameter in addition to the SortAndBindOptions parameter.

You can have overloads that take EITHER a SortAndBindOptions OR an IScheduler parameter, and then the latter version can just call the former version with something like DynamicDataOptions.SortAndBind with { Scheduler: scheduler }.

[RolandPheasant]

Dang. My original PR had the scheduler on SortAndBindOption and I'm now going in circles.

In many respects, it makes sense to have the scheduler on SortAndBindOption as all the new binding operators use these whereas I would rather not add it to the original binding options at that would mean I'd have to retrospectively change those having marked them obsolete.

If you do that way, then you don't need a separate IScheduler parameter in addition to the SortAndBindOptions parameter.
You can have overloads that take EITHER a SortAndBindOptions OR an IScheduler parameter, and then the latter version can just call the former version with something like DynamicDataOptions.SortAndBind with { Scheduler: scheduler }.

Wise words.

I'll sit on it for a day or two, but keeping the overloads to a minimum seems like a good idea.

[JakenVeina]

It seems confusing to have ADDITIONAL "Binding" options at the top level when there is already a section for Binding specific options.

It doesn't seem confusing to me. DynamicDataOptions.BindingScheduler applies to all "Binding" operators, while .SortAndBindOptions.Scheduler applies to just the .SortAndBind() operator. I think there's value to be argued for both, but I definitely wouldn't add DynamicDataOptions.BindingScheduler without retrofitting it into all the existing operators. That would definitely be counter-intuitive. And I'd say that scope of change should be its own PR.

If you do that way, then you don't need a separate IScheduler parameter in addition to the SortAndBindOptions parameter.

Agreed. I see no point in having overloads for both IScheduler and SortAndBindOptions. If there was already an IScheduler one, it would make sense to add one for SortAndBindOptions, and leave the existing one there to avoid breakage, but that's not the case here.

In many respects, it makes sense to have the scheduler on SortAndBindOption as all the new binding operators use these

I think this is probably the way to go. That is, having each operator define its own XXXOptions struct, instead of just adding optional parameters. It makes adding new options much simpler. And, honestly, I think the point about being forced to retrofit some older operators when adding new options is a BENEFIT. It's a constraint we're placing on ourselves to keep the API a little more sane for consumers. I quite literally think there is only one downside to the XXXOptions pattern, and that's the increase in call site size (I.E. every call requires a full allocation of the options struct, regardless of which options are needed). I'm inclined to think this is a cost worth accepting, particularly since operator calls are not hot paths, only the operator innards are.