Skip to content

DynamicData's Switch() overloads silently shadow Observable.Switch() #1148

Description

@dwcullop

Describe the bug 🐞

ObservableCacheEx.Switch() and ObservableListEx.Switch() each declare an overload that is more specific than the standard Rx Observable.Switch<T>() for the shape most callers actually write. Overload resolution picks the DynamicData one silently, and nothing about the call site suggests anything unusual happened.

// src/DynamicData/Cache/ObservableCacheEx.Switch.cs:52
public static IObservable<IChangeSet<TObject, TKey>> Switch<TObject, TKey>(
    this IObservable<IObservable<IChangeSet<TObject, TKey>>> sources)

// src/DynamicData/List/ObservableListEx.Switch.cs:61
public static IObservable<IChangeSet<T>> Switch<T>(
    this IObservable<IObservable<IChangeSet<T>>> sources)

Any expression that produces an IObservable<IObservable<IChangeSet<...>>> binds to these rather than to Rx's Switch<T>, because IChangeSet<...> is a closer match than an open T. That includes the very common:

someObservable
    .Select(_ => cache.Connect())
    .Switch();          // not the operator most people think they are calling

Why it matters

The two operators are not interchangeable. Rx's Switch flattens to the most recent inner sequence and does nothing else. The DynamicData ones maintain an intermediate collection and compute item diffs so that switching away from a source removes the items it contributed. That is genuinely useful when you want it, and pure overhead when you don't.

So anyone who wanted a plain flatten gets change-set diffing machinery instead: extra allocation and bookkeeping per switch, plus whatever semantics the diffing implies for the stream. It is a silent behaviour and performance difference, not a compile error.

This surfaced in #1145, where ObservableCache.Connect() was hitting it internally. Worth stressing that it was found by accident while chasing #1136: nobody spotted it by reading the line, which is rather the point.

Step to reproduce

using var cache = new SourceCache<int, int>(static i => i);

IObservable<IObservable<IChangeSet<int, int>>> nested =
    Observable.Return(cache.Connect());

// Binds to ObservableCacheEx.Switch, not Observable.Switch.
var switched = nested.Switch();

Same shape on the list side with SourceList<T> and ObservableListEx.Switch.

Expected behavior

Calling .Switch() should either land on the operator the caller intended, or make it obvious which one they got. Some options, roughly in order of how disruptive they are:

  • Document it. An XML doc remark on both overloads noting that they shadow Observable.Switch for this shape, and that callers wanting a plain flatten should call Observable.Switch(...) explicitly. Cheapest, and would have saved the time this cost.
  • Add an analyzer warning when the result of the DynamicData overload is used in a way that suggests a plain flatten was meant. More work, but catches it where the mistake is made.
  • Rename the DynamicData overloads to something intention-revealing such as SwitchChangeSets(), leaving .Switch() to resolve to Rx. Cleanest to read, but a breaking change, so it would need to ride a major version.

No strong preference on which, but the current state is a trap and it has now cost us at least once.

DynamicData Version

main

Additional information ℹ️

Related to #1136 and #1145. #1141 removes the internal call site that was affected, so this is no longer an active bug in the library's own code, but the trap remains for anyone consuming it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions