-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
One single global stateless OnCompleted Notification object #662
Comments
Does this really make much of a difference when the In general we probably need to move away from (or be cautious about) the use of |
The reason why I opened this is that I want to make OnCompleted a singleton object in the Scala adaptor. This is possible with the current Java Notifications, but before I do it, I want to check if there will be changes in the Java Notifications, so that I can avoid doing the work twice. I think there might be changes in the Java Notifications because I see some issues:
There's a non-breaking way to solve these issues:
And there's also a breaking (but clean and nice) way to solve these issues:
I'm for the second way, but since both ways are fine for the Scala adaptor, I won't insist more than this and let you decide. And once it's decided, I'll make the changes in the Scala adaptor. |
I agree with Ben, It does not seem to matter much perf-wise to special case Notification.OnCompleted, also since whenever you use Notifications you are already paying a hug interpretative overhead in doing case analysis. But maybe I am missing something? That said, in Scala Notification.OnCompleted is already using a factory method, so if you really want to use a singleton object you can do that in Scala. |
Just tried to make a static OnCompletedNotification but can't do it as it uses generics and must have a type I don't see how to make a static representation even if we want to while having it be type-safe. |
To make it work without casting, we would need a Bottom type, such that we can say |
Done, could you close the issue? |
Instead of creating a new OnCompleted Notification object every time we need one, we could have one global instance, accessible by a factory method on Notification (suggested by @akarnokd here).
Advantages:
object OnCompleted
instead of aclass OnCompleted
which has to wrap a Java OnCompleted object.Maybe we could also get 2) without any changes in Java, but then we might need to define clearly if the
toScalaNotification
/toJavaNotification
methods have to preserve object identity, or onlyequals
.The text was updated successfully, but these errors were encountered: