Operator: Generate #509
Operator: Generate #509
Conversation
|
RxJava-pull-requests #433 SUCCESS |
| * TimeSpan oneSecond = TimeSpan.of(1, TimeUnit.SECONDS); | ||
| * </pre> | ||
| */ | ||
| public final class TimeSpan implements Comparable<TimeSpan> { |
There was a problem hiding this comment.
We have thus far avoided doing this as it does not match the Java way ... even though long time, TimeUnit unit is ugly.
Is there a strong reason for this other than syntax?
There was a problem hiding this comment.
I designed it this way so no timing information is lost. The alternative is to set a base unit like .NET (e.g., 100 nanoseconds per tick). It is up to the RxJava project to decide what to select.
There was a problem hiding this comment.
The timing information isn't lost in Java either ... it's just that it's always sent around as 2 arguments.
It's very obnoxious ... but that's how it's done everywhere in Java and we have thus far avoided moving away from the idiomatic (though worse) Java approach.
As an example, look at all of the methods in the Scheduler interface that accept long delayTime, TimeUnit unit.
If we're going to move away from idiomatic Java it would need to be done across the entire library for consistency.
Obviously it's a nicer class and others have done this before: https://www.google.com/search?client=safari&rls=en&q=java+TimeStpan&ie=UTF-8&oe=UTF-8#q=java+TimeSpan&rls=en&safe=off
What do others think we should do about this? @jmhofer @abersnaze @headinthebox @samuelgruetter @zsxwing @mttkay @JakeWharton @adriancole Do we make the breaking change to the entire library and change all uses of long delayTime, TimeUnit unit to TimeSpan?
There was a problem hiding this comment.
I think changing Func1<TState, TimeSpan> timeSelector to two parameters Func1<TState, Long> timeSelector, TimeUnit unit is better. And as there is no DateTimeOffset in Java, we do not need generateAbsoluteTime.
There was a problem hiding this comment.
Java 8 has Duration. Not sure it's worth establishing a library-specific convention when the platform has addressed the need (although it will be a while before we can use it, of course).
There was a problem hiding this comment.
For what it's worth, I would give a +1 for any solution that does not split
up value and unit into two separate parameters (as Java does by default),
as they are clearly logically coupled.
I would give another +1 for making whatever solution is settled for
consistent across the code base.
Beyond that, I don't really feel strong about one solution or another.
There was a problem hiding this comment.
We can return rx.util.TimeInterval<TimeUnit> so no new class needs to be introduced, but since TimeInterval is specified as having a millisecond value, it would confuse things.
One option is always there: "when in doubt, leave it out". So there won't be any timed overload, and if particular clients require one, they can write their own time generator and zip() it with generate()
There was a problem hiding this comment.
Airlift has a good duration/timespan abstraction if I were to poach one
from an existing project today.
On Tue, Nov 26, 2013 at 10:28 AM, akarnokd notifications@github.com wrote:
In rxjava-core/src/main/java/rx/util/TimeSpan.java:
+package rx.util;
+
+import java.util.concurrent.TimeUnit;
+
+/**
- * Represents a time value and time unit.
- *
- * Rx.NET note: System.TimeSpan has a fixed unit of measure of 100 nanoseconds
- * per value; the Java way is to specify the TimeUnit along with the time value.
- *
- * Usage:
- *
- * TimeSpan oneSecond = TimeSpan.of(1, TimeUnit.SECONDS);
- *
- */
+public final class TimeSpan implements Comparable {We can return rx.util.TimeInterval so no new class needs to be
introduced, but since TimeInterval is specified as having a millisecond
value, it would confuse things.One option is always there: "when in doubt, leave it out". So there won't
be any timed overload, and if particular clients require one, they can
write their own time generator and zip() it with generate()—
Reply to this email directly or view it on GitHubhttps://github.com//pull/509/files#r7934024
.
There was a problem hiding this comment.
Java code using Observables is bulky anyways because of anonymous functions, so it doesn't matter if durations are ugly, too ;-) I think RxJava's "mission" should be the a base compatible with everything, and will mostly be used from other languages. In Scala, for instance, we use Duration, and I'm very happy that it is compatible with TimeUnit. So I'd say first priority is compatibility, second is that it's "nice". But if someone comes with a solution which provides both, all the better.
There was a problem hiding this comment.
There's no reason to make a bad api in Java just because it is Java :-)
On Tue, Nov 26, 2013 at 11:21 AM, samuelgruetter
notifications@github.comwrote:
In rxjava-core/src/main/java/rx/util/TimeSpan.java:
+package rx.util;
+
+import java.util.concurrent.TimeUnit;
+
+/**
- * Represents a time value and time unit.
- *
- * Rx.NET note: System.TimeSpan has a fixed unit of measure of 100 nanoseconds
- * per value; the Java way is to specify the TimeUnit along with the time value.
- *
- * Usage:
- *
- * TimeSpan oneSecond = TimeSpan.of(1, TimeUnit.SECONDS);
- *
- */
+public final class TimeSpan implements Comparable {Java code using Observables is bulky anyways because of anonymous
functions, so it doesn't matter if durations are ugly, too ;-) I think
RxJava's "mission" should be the a base compatible with everything, and
will mostly be used from other languages. In Scala, for instance, we use
Durationhttp://www.scala-lang.org/api/current/index.html#scala.concurrent.duration.Duration,
and I'm very happy that it is compatible with TimeUnit. So I'd say first
priority is compatibility, second is that it's "nice". But if someone comes
with a solution which provides both, all the better.—
Reply to this email directly or view it on GitHubhttps://github.com//pull/509/files#r7936030
.
|
This PR needs to be rebased so it can merge ... along with some other questions/comments above. |
| return new OnSubscribeFunc<R>() { | ||
| @Override | ||
| public Subscription onSubscribe(final Observer<? super R> observer) { | ||
| return scheduler.schedule(initialState, new Func2<Scheduler, TState, Subscription>() { |
There was a problem hiding this comment.
Is it OK that creating a special timeSelector which always returns 0 to remove these duplicated codes?
|
Closing this PR as both this and the other variant (#519) need further work. |
This commit adds support for adding tags to Retry, CircuitBreaker, Ratelimiter and Bulkhead functionality. These tags can then be used by other modules for example Prometheus, MicroMeter, etc.
…moryCircuitBreakerRegistry.
Operator Generate (6 variants) for Issue #49.