Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vs IObservable<T> #10

Closed
neuecc opened this issue Jan 4, 2024 · 8 comments
Closed

vs IObservable<T> #10

neuecc opened this issue Jan 4, 2024 · 8 comments

Comments

@neuecc
Copy link
Member

neuecc commented Jan 4, 2024

The current API uses a custom definition that differs from IObservable<T> and is therefore incompatible as is (conversion is possible with AsIObservable).
Whether we should go with a custom definition or conform to IObservable<T>.

@mediahype
Copy link

Hi I am totally new to the Subject - so what i'm saying is probably totally wrong - but as I am reading this artice: https://reactivex.io/intro.html

At the bottom of the page it says:

The Observable type adds two missing semantics to the Gang of Four’s Observer pattern, to match those that are available in the Iterable type:

  1. the ability for the producer to signal to the consumer that there is no more data available (a foreach loop on an Iterable completes and returns normally in such a case; an Observable calls its observer’s onCompleted method)
  1. the ability for the producer to signal to the consumer that an error has occurred (an Iterable throws an exception if an error takes place during iteration; an Observable calls its observer’s onError method)

So isn't the question here, if IObservable has these semantics?

@mediahype
Copy link

But another question I would also ask is what would be the simplest for the library?

@mediahype
Copy link

Btw if you rename this, you should definitely call it UnitedRx ;-)

@neuecc
Copy link
Member Author

neuecc commented Jan 18, 2024

The reason for the change is described in the ReadMe.

@vangogih
Copy link

I think it would be better to create a different interface and write AsSystemObservable() extension.

Reasons:

  • Speed of changes are different for both interfaces
  • They looks quite similar but they are different. Because they are parts of different runtime. And one runtime might not support other. And I think this difference should have highlighted by separating, nor confrorming
  • If you inherit your interface from System you will have to support back compatibility with it. Do you rly want it?)

Possible problems:

  • Migration issues from UniRx to R3. But how many people will migrate from UniRx to R3? Anyway we can always write script file migrators to solve this problem.

@kochounoyume
Copy link
Contributor

kochounoyume commented Jan 22, 2024

This comment is a little off from the original issue raised, but I would like to see an extension method that can execute AsSystemObservable() and AsAsUnitObservable() at the same time if possible.

The purpose of converting to IObservable<T> type is for cases where you want to limit the use to the Subscribe method only, and in many cases you will want to publish it as IObservable<Unit> type then.

@vangogih
Copy link

The purpose of converting to IObservable<T> type is for cases where you want to limit the use to the Subscribe method only, and in many cases you will want to publish it as IObservable<Unit> type then.

Hm, I suggested just a name, format. Of course we must support generic parametrization.

And I meant, of course AsSystemObservable<T>() ,because origin interface we want to cast has generic parametrization.

@kochounoyume
Copy link
Contributor

Of course we must support generic parametrization.

I strongly agree with that.

I was only suggesting the addition of an API, not that the current API should be rearranged.

I also believe that my proposal is a watershed.

It comes down to this iuuse agenda in the end.

If we choose not to stick with IObservable, we should present the importance of using IObservable.

My proposal is based on the assumption that the significance of IObservable use is the limitation of Subscribe-only, and beyond that, I think it is better not to adopt it if it defines a different significance.

@neuecc neuecc closed this as completed Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants