Removes Observable methods that also have a Scheduler parameter counterpart

[jbripley]

Fixes #40 by removing Observable methods that also have a Scheduler parameter counterpart, by providing a default value for the Scheduler parameter.

• Changes some methods to require parentheses (timestamp, timeInterval)

[samuelgruetter]

Now that's much nicer :-)

Thanks also for the list in the commit message of 362db2a. The "multiple overloaded alternatives of method xxx define default arguments" error is very annoying: It's not possible to have several overloads with default arguments, even if there's no ambiguity. There are some answers on stackoverflow why it doesn't work, but I'm not quite satisfied by them...

The problem is that now most methods have nice default scheduler arguments, but some do not but still silently just choose a default scheduler. This is very misleading.

So far I see the following solutions to the default schedulers problem:

1. this PR
2. this PR, but rename methods so that each method has at most one overload with a default scheduler
3. this PR, but delete all methods which silently choose a default scheduler
4. Implicit Schedulers #16
5. leave as is

I'm not happy with any of these. Any ideas?

[headinthebox]

but some do not but still silently just choose a default scheduler.

Can we get a list of those?

[samuelgruetter]

Can we get a list of those?

It's the first list of the three lists in the commit message of jbripley@362db2a.

[jbripley]

In my order of preference:

2: Breaks a lot of existing methods, but is the most straightforward one to maintain/understand.
1: + make sure to always document which default scheduler is used. This is the most pragmatic solution , since it doesn't break the current API, much (timestamp Vs timestamp()).
4: #16 is harder to understand/follow, but is pretty close to how the Future and other Scala API's work.
3: Will lead to a lot of boilerplate specifying which Scheduler to use, since most times now the default Scheduler is the one you want.
5: Leads to a needlessly big API to maintain as well as the other things listed in #40.

[samuelgruetter]

"Overloading is evil" (https://groups.google.com/forum/#!topic/dotty-internals/T5UBRkVc8Ks).
Just thinking out loud: What if we get rid of all overloading? This would also make it easier to talk about a given operator: Instead of saying "the overload of slidingBuffer which takes an Observable of Openings and an Function from Openings to Observable of Any" you could just say "slidingXxxBuffer"...

[jbripley]

This is the minimal number of changes that need to be made, together with previous commits, to allow for a default Scheduler value on all Observable methods that can take a Scheduler:

Current Method	Replacing Method
tumblingBuffer(Duration)	tumblingTimeBuffer(Duration, Scheduler)
tumbling(Duration, Int)	def tumblingTimeCount(Duration, Int, Scheduler)
replay(Int, Duration)	def replayBufferSizeTime(Int, Duration, Scheduler)
replay(Int)	def replayBufferSize(Int, Scheduler)
replay(Duration)	def replayTime(Duration, Scheduler)
takeRight(Int, Duration)	def takeCountTimeRight(Int, Duration, Scheduler)
timeout[U >: T](Duration, Observable[U])	def timeoutFallback[U >: T](Duration, Observable[U], Scheduler)
repeat	repeat(Scheduler)
repeat(Long)	repeatCount(Long, Scheduler)
timer(Duration, Duration)	timerInitialDelayPeriod(Duration, Duration, Scheduler)

I've been thinking about these methods, as well

No default Scheduler defined in RxJava:
def sliding(Duration, Duration, Int, Scheduler)
def replay[R](Observable[T] => Observable[R], Duration, Scheduler)

Should we just come up with a reasonable default Scheduler for these and differ slightly from RxJava? ComputationScheduler should work in both cases.

[samuelgruetter]

@jbripley thanks for the table, I think that's probably the way to go. Except for the new method names, I don't particularly like them, but can't think of any better names right now...

Another idea would be to fight the problem with even more default arguments: For repeat, for instance, we could have something like

def repeat(count: Int = Infinity, scheduler: Scheduler  = TrampolineScheduler())

(there's no integer Infinity in Scala, but we could use -1, an Option, null or another hack...)

[jbripley]

No particularly happy with the new method names myself. Maybe something longer and more descriptive like this is better:

Current Method	Replacing Method
tumblingBuffer(Duration)	tumblingBufferUsingTime(Duration, Scheduler)
tumbling(Duration, Int)	def tumblingUsingTimeAndCount(Duration, Int, Scheduler)
replay(Int, Duration)	def replayWithBufferSizeAndTime(Int, Duration, Scheduler)
replay(Int)	def replayWithBufferSize(Int, Scheduler)
replay(Duration)	def replayUsingTime(Duration, Scheduler)
takeRight(Int, Duration)	def takeRightWithCountAndTime(Int, Duration, Scheduler)
timeout[U >: T](Duration, Observable[U])	def timeoutUsingFallback[U >: T](Duration, Observable[U], Scheduler)
repeat	repeat(Scheduler)
repeat(Long)	repeatWithCount(Long, Scheduler)
timer(Duration, Duration)	timerWithInitialDelayAndPeriod(Duration, Duration, Scheduler)

Another idea would be to fight the problem with even more default arguments

When there's no natural default/sentinel value, Option is the way to go IMO:

def repeat(count: Option[Int] = None, scheduler: Scheduler  = TrampolineScheduler())

[headinthebox]

Brrr names like takeRightWithCountAndTime and timerWithInitialDelayAndPeriod are just ridiculous. Let's not make RxScala look like COBOL. I will veto any such changes.

Then it is better to not change anything.

[jbripley]

Maybe I only suggested those really horrible method names so someone would feel compelled to come up with better ones :)

[headinthebox]

As in, keep things as they are. If a refactor forces you to invent new names, it makes the API worse.

[jbripley]

As in, keep things as they are.

I'm confused. Do you mean to keep the changes as currently in this PR or to not do these changes at all?

[headinthebox]

No changes at all. This makes the API a lot worse. We will need to take a lot more time to figure this out.

Sometimes an approach looks promising, but turns out to be a dead end. We have found a textbook example with this PR.

[samuelgruetter]

I went again through the list of methods which have multiple overloads taking a Scheduler, i.e. those which would require renaming if we wanted to use default arguments for Schedulers. I found that they can be split into two categories:

1. Those methods to which the user has to indicate the maximum size of an Observable or Buffer. Such a max size can be a maximum number of elements, or a maximum amount of time, or both. The conflicting overloads are:

• tumbling(Duration)/tumbling(Duration, Int)
• tumblingBuffer(Duration)/tumblingBuffer(Duration, Int)
• sliding(Duration)/sliding(Duration, Int)
• slidingBuffer(Duration)/slidingBuffer(Duration, Int)
• replay(Int)/replay(Duration)/replay(Int, Duration)
• takeRight(Int, Duration)/takeRight(Duration)

So far, all attempts to rename these methods to avoid the conflicts resulted in ugly long COBOL-like names.

I also tried to use something like a SizeBound class (see samuelgruetter@38f8319), so that we would need only one method per operator, but it made the API much worse.

2. Those methods which do not need such a "maximum size argument". It turns out that renaming these to avoid the conflicts would not be too bad:

Current Method	After renaming
timeout(Duration)	timeoutError(Duration)
timeout(Duration, Observable[U])	timeoutResumeNext(Duration, Observable[U])
repeat	infiniteRepeat()
repeat(Long)	repeat(Long) [unchanged]
timer(Duration)	timer(Duration) [unchanged]
timer(Duration, Duration)	timeTicks(Duration, Duration)

As you can see, this post does not propose any solution. But I still have some hope that we might be able to use default arguments for default Schedulers, so I'm posting this to initiate others to think about it ;-)

[jbripley]

I like alt. 2. I could update this PR with those changes, and put in default schedulers for the method that didn't work properly before, if @headinthebox agrees.

[samuelgruetter]

@jbripley please don't start coding before we have a complete solution. My above post only gives renamings for the method category 2), but what would you do with those listed under 1)?

[jbripley]

Sorry, I read through your comment a bit too quickly over lunch and misinterpreted the two groups. I'll try to think of something for group 1, but Scala doesn't give us that many options with regards to overloaded methods.

A version of the SizeBound using a sealed trait will make that a bit nicer, but not by much:

sealed trait Bound
case class CountBound(maxCount: Int) extends Bound
case class TimeBound(maxTime: Duration) extends Bound
case class SizeBound(maxCount: Int, maxTime: Duration) extends Bound