fsprojects / FSharp.Control.Reactive

Extensions and wrappers for using Reactive Extensions (Rx) with F#.
http://fsprojects.github.io/FSharp.Control.Reactive
Other
284 stars 58 forks source link

Consider removing isEmpty function #68

Closed bordoley closed 8 years ago

bordoley commented 9 years ago

This method is dubious at best. The current implementation does a reference compare to the object returned by Observable.Empty() afaict.

  /// Determines whether the given observable is empty 
    let isEmpty source = source = Observable.Empty()

For instance this could easily result in a false result if an Observable was created from an empty array, or if the implementation of Observable.Empty() was changed not to return a singleton instance.

panesofglass commented 9 years ago

Rather than removing this, do you think we could fix it? Is it worth fixing?

bordoley commented 9 years ago

I don't think you can sanely fix the api. Consider an IObservable of mouse events or really any hot observable. What does it mean to be empty? Does empty really mean completed, and during the time I was observing it, no events were generated?

panesofglass commented 9 years ago

Good point

cloudRoutine commented 8 years ago

The implementation should have called Observable.IsEmpty from System.Reactive.Linq The docstring for isEmpty should be more clear stating

/// isEmpty returns an Observable that emits true if and only if the source 
/// Observable completes  without emitting any items. 
bordoley commented 8 years ago

@cloudRoutine good call on changing the method signature to return an observable. That makes sense. Thanks.