Support creating unbounded ReplaySubject

[fpillet]

There was previously no way to externally instantiate the ReplayAll concrete class

[kzaher]

Hi @fpillet , @ashfurrow

Yeah, I was thinking about should I expose that overload or not. I think that if we would want to expose that, we would want to have interface something like this

// docs
 public static func create() -> ReplaySubject<Element> {
    return ReplayAll()  
}

... because it doesn't really have an upper bound. I am little worried that people will just use this unbounded one without thinking.

The problem with using unbounded version is that you are potentially creating a memory leak (you need to store all generated elements).

If you guys want us to expose this, I guess I'm fine with it.

I think we would also need to add unit tests for this unlimited version like we've done for version with 1 and many :)

[ashfurrow]

Yeah, I can understand the concerns. Introducing friction to limit accidental use sounds 👍 to me.

For my case, at least, 99% of the time I'll be just sending Void(), and only as fast as users can tap a button, so I'm not too concerned 😄

[fpillet]

I usually find that having to specify an upper bound is cumbersome because most of the time, your code will behave reasonably well and not fill up memory. When this happens, and when using a ReplaySubject, it seems to me that the developer has to assume the risk.

This said, maybe you'll want it exposed in a more explicit way:

public static func createUnbounded() -> ReplaySubject<Element> {
    return ReplayAll()
}

Either way is fine with me. I can change my PR or let you do it your way. Let me know.

As to tests, yes for sure will have to figure out some reasonable tests for this. Want them added to the PR?

[kzaher]

@fpillet yeah, I have thought about createUnbounded. I really like that name a lot, so let's use that.

I think it jiggles reader's mind more, and hoping they will investigate what unbounded means :)

I think it would be great if you could add unit tests to this PR before merging this so I can remove this from my mental stack for good :)

Just analogous tests to ones we have for 1 and many version, nothing fancy :)

[fpillet]

@kzaher : ok will do this w-e. I also need to add an ObservableType extension function in Observable+binding.swift, along with replay(bufferSize: Int) -> ConnectableObservable<ReplaySubject<E>>. I suggest we add replayAll() here unless there is a more appropriate Rx-mandated name.

Opinions?

[kzaher]

@fpillet I think maybe just exposing that subject for now is ok.

I am curios in what case would somebody want that replayAll overload :)

Since that's just a convenience for multicast anyway, we aren't limiting anyone, but on the other hand not advertising it :)

[kzaher]

I think this looks reasonable :)

I'll probably add a warning about memory allocation in docs to further warn the users :)