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

A better name of collect #63

Closed
zsxwing opened this issue Nov 18, 2014 · 6 comments
Closed

A better name of collect #63

zsxwing opened this issue Nov 18, 2014 · 6 comments
Labels

Comments

@zsxwing
Copy link
Member

zsxwing commented Nov 18, 2014

RxJava changed collect to Observable<R> collect(Func0<R> stateFactory, final Action2<R, ? super T> collector) in https://github.com/ReactiveX/RxJava/pull/1884/files#diff-c3bbc6e9e497930d46361b0b8140431cR3471 to fix ReactiveX/RxJava#1831

RxScala has the same issue and needs a similar operator too. ReactiveX/RxJava#1835 is a great fix for RxScala but has an ambiguity issue: ReactiveX/RxJava#1884

To solve this issue, here are 3 options:

  1. Add collect into RxScala to call RxJava's collect. However, RxScala has a same name method: def collect[R](pf: PartialFunction[T, R]): Observable[R]. It may confuse people because they are totally different.

  2. Change foldLeft to

  def foldLeft[R](initialValue: => R)(accumulator: (R, T) => R): Observable[R] = {
    val func = new rx.functions.Func0[Array[R]] {
      override def call(): Array[R] = Array(initialValue)
    }
    toScalaObservable[R](asJavaObservable.collect(func, new Action2[Array[R],T]{
      def call(t1: Array[R], t2: T): Unit = {
        t1(0) = accumulator(t1(0),t2)
      }
    }).map(_(0)))
  }
  1. Give another name for RxJava collect.

I prefer 3). Any suggestion for the new name?

@samuelgruetter
Copy link
Collaborator

I'll probably also prefer to have another name for the RxJava collect in RxScala. But it seems that similar problems arise with Scan and Reduce, and they're not yet solved in RxJava, so maybe wait until we know what's happening in RxJava?

@zsxwing
Copy link
Member Author

zsxwing commented Jan 27, 2015

Any suggestion for the name? @samuelgruetter @headinthebox @jbripley

@samuelgruetter
Copy link
Collaborator

Maybe foldMutable?

@zsxwing
Copy link
Member Author

zsxwing commented Jan 28, 2015

I think finally we should change scan and foldLeft to

def scan[R](initialValue: => R)(accumulator: (R, T) => R): Observable[R]
def foldLeft[R](initialValue: => R)(accumulator: (R, T) => R): Observable[R]

Adding RxJava collect to RxScala won't fix the method signatures.

For foldLeft, we can support a by-name initialValue using RxJava collect. See zsxwing@b59fcd2
But for scan, now I don't find out a way except reimplementing it in RxScala.

Should we keep the current improper signatures and add a method to give users alternative option, or fix them?

@samuelgruetter
Copy link
Collaborator

I think finally we should change scan and foldLeft to

def scan[R](initialValue: => R)(accumulator: (R, T) => R): Observable[R]
def foldLeft[R](initialValue: => R)(accumulator: (R, T) => R): Observable[R]

I don't agree on this, because scan and foldLeft are meant to be used with immutable data structures for the state, so it's no problem to reuse the same initial value several times. And if you really need a new one, you can either use the RxScala equivalent of RxJava's collect that we're going to add, or you can use Observable.defer.

@dhoepelman
Copy link
Collaborator

Due to project EOL status, this improvement will not be made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants