27

In this answer to a question about Subject<T> Enigmativity mentioned:

as an aside, you should try to avoid using subjects at all. The general rule is that if you're using a subject then you're doing something wrong.

I often use subjects as backing fields for IObservable properties, which would have probably been .NET events in the days before Rx. e.g. instead of something like

public class Thing
{
    public event EventHandler SomethingHappened;

    private void DoSomething()
    {
        Blah();
        SomethingHappened(this, EventArgs.Empty);
    }
}

I might do

public class Thing
{
    private readonly Subject<Unit> somethingHappened = new Subject<Unit>();
    public IObservable<Unit> SomethingHappened
    {
        get { return somethingHappened; }
    }

    private void DoSomething()
    {
        Blah();
        somethingHappened.OnNext(Unit.Default);
    }
}

So, if I want to avoid using Subject what would be the correct way of doing this kind of thing? Or I should I stick to using .NET events in my interfaces, even when they'll be consumed by Rx code (so probably FromEventPattern)?

Also, a bit more details on why using Subject like this is a bad idea would be helpful.

Update: To make this question a bit more concrete, I'm talking about using Subject<T> as a way to get from non-Rx code (maybe you're working with some other legacy code) into the Rx world. So, something like:

class MyVolumeCallback : LegacyApiForSomeHardware
{
    private readonly Subject<int> volumeChanged = new Subject<int>();

    public IObservable<int> VolumeChanged
    {
        get
        {
            return volumeChanged.AsObservable();
        }
    }

    protected override void UserChangedVolume(int newVolume)
    {
        volumeChanged.OnNext(newVolume);
    }
}

Where, instead of using events, the LegacyApiForSomeHardware type makes you override virtual methods as a way of getting "this just happened" notifications.

Community
  • 1
  • 1
Wilka
  • 28,701
  • 14
  • 75
  • 97

4 Answers4

17

For one thing, someone can cast the SomethingHappened back to an ISubject and feed things into it from the outside. At the very least, apply AsObservable to it in order to hide the subject-ness of the underlying object.

Also, subject broadcasting of callbacks doesn't do strictly more than a .NET event. For example, if one observer throws, the ones that are next in the chain won't be called.

    static void D()
    {
        Action<int> a = null;

        a += x =>
        {
            Console.WriteLine("1> " + x);
        };

        a += x =>
        {
            Console.WriteLine("2> " + x);

            if (x == 42)
                throw new Exception();
        };

        a += x =>
        {
            Console.WriteLine("3> " + x);
        };

        a(41);
        try
        {
            a(42);  // 2> throwing will prevent 3> from observing 42
        }
        catch { }
        a(43);
    }

    static void S()
    {
        Subject<int> s = new Subject<int>();

        s.Subscribe(x =>
        {
            Console.WriteLine("1> " + x);
        });

        s.Subscribe(x =>
        {
            Console.WriteLine("2> " + x);

            if (x == 42)
                throw new Exception();
        });

        s.Subscribe(x =>
        {
            Console.WriteLine("3> " + x);
        });

        s.OnNext(41);
        try
        {
            s.OnNext(42);  // 2> throwing will prevent 3> from observing 42
        }
        catch { }
        s.OnNext(43);
    }

In general, the caller is dead once an observer throws, unless you protect every On* call (but don't swallow exceptions arbitrarily, as shown above). This is the same for multicast delegates; exceptions will swing back at you.

Most of the time, you can achieve what you want to do without a subject, e.g. by using Observable.Create to construct a new sequence. Such sequences don't have an "observer list" that results from multiple subscriptions; each observer has its own "session" (the cold observable model), so an exception from an observer is nothing more than a suicide command in a confined area rather than blowing yourself up in the middle of a square.

Essentially, subjects are best used at the edges of the reactive query graph (for ingress streams that need to be addressable by another party that feeds in the data, though you could use regular .NET events for this and bridge them to Rx using FromEvent* methods) and for sharing subscriptions within a reactive query graph (using Publish, Replay, etc. which are Multicast calls in disguise, using a subject). One of the dangers of using subjects - which are very stateful due to their observer list and potential recording of messages - is to use them when trying to write a query operator using subjects. 99.999% of the time, such stories have a sad ending.

casperOne
  • 73,706
  • 19
  • 184
  • 253
Bart De Smet
  • 566
  • 3
  • 3
  • Does "subjects are best used at the edges of the reactive query graph" mean that my use Subject is ok if "DoSomething()" is a callback from a legacy API that is called when the user performs some action, such as changing a volume control. – Wilka Aug 22 '12 at 10:21
12

In an answer on the Rx forum, Dave Sexton (of Rxx), said as part an answer to something:

Subjects are the stateful components of Rx. They are useful for when you need to create an event-like observable as a field or a local variable.

Which is exactly what's happening with this question, he also wrote an in-depth follow up blog post on To Use Subject Or Not To Use Subject? which concludes with:

When should I use a subject?

When all of the following are true:

  • you don't have an observable or anything that can be converted into one.
  • you require a hot observable.
  • the scope of your observable is a type.
  • you don't need to define a similar event and no similar event already exists.

Why should I use a subject in that case?

Because you've got no choice!

So, answering the inner question of "details on why using Subject like this is a bad idea" - it's not a bad idea, this is one of the few places were using a Subject is the correct way to do things.

Wilka
  • 28,701
  • 14
  • 75
  • 97
2

While I can't speak for Enigmativity directly, I imagine it's because it's very low-level, something you don't really need to use directly; everything that's offered by the Subject<T> class can be achieved by using the classes in the System.Reactive.Linq namespace.

Taking the example from the Subject<T> documentation:

Subject<string> mySubject = new Subject<string>();

//*** Create news feed #1 and subscribe mySubject to it ***//
NewsHeadlineFeed NewsFeed1 = new NewsHeadlineFeed("Headline News Feed #1");
NewsFeed1.HeadlineFeed.Subscribe(mySubject);

//*** Create news feed #2 and subscribe mySubject to it ***//
NewsHeadlineFeed NewsFeed2 = new NewsHeadlineFeed("Headline News Feed #2");
NewsFeed2.HeadlineFeed.Subscribe(mySubject);

This is easily achieved with the Merge extension method on the Observable class:

IObservable<string> feeds = 
    new NewsHeadlineFeed("Headline News Feed #1").HeadlineFeed.Merge(
    new NewsHeadlineFeed("Headline News Feed #2").HeadlineFeed);

Which you can then subscribe to normally. Using Subject<T> just makes the code more complex. If you're going to use Subject<T> then you should be doing some very low-level processing of observables where the extension methods fail you.

casperOne
  • 73,706
  • 19
  • 184
  • 253
  • What about when working with a legacy library that you want to make available to the Rx world? I've updated my question with more concrete example. – Wilka Aug 24 '12 at 09:21
2

One approach for classes which have simple one-off events, is to provide a ToObservable method which creates a meaningful cold observable based on an event. This is more readable than using the Observable factory methods, and allows developers who don't use Rx to make use of the API.

    public IObservable<T> ToObservable()
    {
        return Observable.Create<T>(observer =>
        {
            Action notifier = () =>
            {
                switch (Status)
                {
                    case FutureStatus.Completed:
                        observer.OnNext(Value);
                        observer.OnCompleted();
                        break;

                    case FutureStatus.Cancelled:
                        observer.OnCompleted();
                        break;

                    case FutureStatus.Faulted:
                        observer.OnError(Exception);
                        break;                        
                }
            };

            Resolve += notifier;

            return () => Resolve -= notifier;

        });
    }
Asti
  • 12,447
  • 29
  • 38