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

Merge of Pull 657: Average and Sum #698

Merged
merged 4 commits into from
Dec 27, 2013

Conversation

benjchristensen
Copy link
Member

No description provided.

akarnokd and others added 4 commits December 23, 2013 19:10
- avoiding methods with `index`
- resultSelector overload is same as `reduce(seed, accumulator).map(resultSelector)`
benjchristensen added a commit that referenced this pull request Dec 27, 2013
@benjchristensen benjchristensen merged commit 92ba6e7 into ReactiveX:master Dec 27, 2013
@benjchristensen benjchristensen deleted the pull-657-merge branch December 27, 2013 21:13
@cloudbees-pull-request-builder

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

@DavidMGross
Copy link
Collaborator

I'm not crazy about the naming:

average() / averageInteger(f)
averageDoubles() / averageDouble(f)
averageFloats() / averageFloat(f)
averageLongs() / averageLong(f)

It doesn't seem to have much rhyme or reason to it. Why are the versions
that take functions singular and the versions that don't are plural (e.g.
"Long" vs "Longs")? Why is the function paired up with averageInteger(f)
just called average() instead of averageIntegers(), when all the other
functions follow that pattern?

On Fri, Dec 27, 2013 at 1:18 PM, CloudBees pull request builder plugin <
notifications@github.com> wrote:

RxJava-pull-requests #615https://netflixoss.ci.cloudbees.com/job/RxJava-pull-requests/615/SUCCESS
This pull request looks good


Reply to this email directly or view it on GitHubhttps://github.com//pull/698#issuecomment-31280530
.

David M. Gross
PLP Consulting

@akarnokd
Copy link
Member

  • I wanted to avoid overload problems in other languages and simply named the ops differently.
  • I wondered why average() didn't include the type in the name. Is it more common to average a stream of ints?
  • I wondered why the originals where plurals in the first place.

@benjchristensen
Copy link
Member Author

I wanted to avoid overload problems in other languages and simply named the ops differently.

This is actually a problem with type-erasure, not the language interop.

This code ...

public static Observable<Integer> average(Observable<Integer> source) {
        return OperationAverage.average(source);
}

public static Observable<Double> average(Observable<Double> source) {
        return OperationAverage.average(source);
}

Results in this error:

Method average(Observable) has the same erasure average(Observable) as another method in type Observable

@benjchristensen
Copy link
Member Author

We can rename the methods. Doing so now.

benjchristensen added a commit to benjchristensen/RxJava that referenced this pull request Dec 27, 2013
@benjchristensen
Copy link
Member Author

Names standardized here: benjchristensen@f8fc1cb

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.

None yet

4 participants