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

Adding utility functions for observables of strings useful for processing non blocking IO. #516

Merged
merged 4 commits into from
Dec 3, 2013

Conversation

abersnaze
Copy link
Contributor

I started developing this for use with the rxjava-apache-http contrib module to safely and scalable decode infinite observables of byte arrays into strings but thought it should live in core because it could be useful for non blocking disk IO.

I did not make this part of rx.Observable because these operators only work on observables of string. The drawback is that discoverability will be harder.

@cloudbees-pull-request-builder

RxJava-pull-requests #441 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #442 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #451 FAILURE
Looks like there's a problem with this pull request

@benjchristensen
Copy link
Member

Looking at this more this seems to fit better as a contrib module. Nowhere else in RxJava do we have utility classes for doing specific types of data manipulation. Once we have String manipulation we'll open the door for all kinds of others. Similar to how Swing and Android utilities are in a separate jar, should we have a data-transformations contrib module where this stuff goes?

@abersnaze
Copy link
Contributor Author

There are these four that operate on Observables of numbers.

rx.Observable.sum(Observable)
rx.Observable.sumDoubles(Observable)
rx.Observable.sumFloats(Observable)
rx.Observable.sumLongs(Observable)

@headinthebox
Copy link
Contributor

The various aggregation operators like sum are unfortunately monomorphic because Java (and .NET) lack a type like Scala's http://www.scala-lang.org/api/current/index.html#scala.math.Numeric. However, if you squint your eyes, the various aggregates are generics.

I agree with Ben that it is best to keep the core as polymorphic and tight as possible, that is operators on Observable where T is a set of types > 1 and have a (hopefully) thriving ecosystem of "domain specific" operations to a contrib module.

There are a couple of tricks that we can pull to still allow for a fluent coding style.

@abersnaze
Copy link
Contributor Author

I wasn't trying to highlight how monomorphic aspect but that It seems inconsistent to the accumulation of java.lang.Number (sum) to exist in the core but not the accumulation of java.lang.String (concat) to also be part of the core.

I'll rejigger the PR to put it in a new module so this can be closed.

@cloudbees-pull-request-builder

RxJava-pull-requests #455 FAILURE
Looks like there's a problem with this pull request

import rx.util.functions.Func1;
import rx.util.functions.Func2;

public class AssertObservable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still inside core and is solely used for testing. Perhaps if it's in core and for testing it should be in the /src/test/ folder?

If it's intended for use in production code then perhaps another contrib module for testing capabilities?

@benjchristensen
Copy link
Member

Much better ... just a question on the AssertObservable class that doesn't feel like it's in the right place.

@cloudbees-pull-request-builder

RxJava-pull-requests #469 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

RxJava-pull-requests #470 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

RxJava-pull-requests #471 SUCCESS
This pull request looks good

benjchristensen added a commit that referenced this pull request Dec 3, 2013
Adding utility functions for observables of strings useful for processing non blocking IO.
@benjchristensen benjchristensen merged commit bd0c47a into ReactiveX:master Dec 3, 2013
@abersnaze abersnaze deleted the string-observable branch January 6, 2014 21:54
rickbw pushed a commit to rickbw/RxJava that referenced this pull request Jan 9, 2014
Adding utility functions for observables of strings useful for processing non blocking IO.
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

Successfully merging this pull request may close these issues.

4 participants