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

Implemented the "Operator: Min and MinBy" and "Operator: Max and MaxBy" #478

Merged
merged 4 commits into from
Nov 12, 2013

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Nov 8, 2013

Hi, this PR implemented the Operator: Min and MinBy #63 and Operator: Max and MaxBy #61. Every operator has 2 variants, one for Comparable, another for Comparator. Please take a look. Thanks!

@cloudbees-pull-request-builder

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

@samuelgruetter
Copy link
Contributor

OperationMin and OperationMax are exactly the same, except that < and > are swapped. Can't you implement one in terms of the other? Or create a general one which takes +1 or -1 to know whether we want < or >?

@zsxwing
Copy link
Member Author

zsxwing commented Nov 11, 2013

Is the following solution OK?

    public static <T extends Comparable<T>> Observable<T> min(
            Observable<T> source) {
        return minMax(source, -1);
    }

    public static <T extends Comparable<T>> Observable<T> max(
            Observable<T> source) {
        return minMax(source, 1);
    }

    public static <T extends Comparable<T>> Observable<T> minMax(
            Observable<T> source, final long flag) {
        return source.reduce(new Func2<T, T, T>() {
            @Override
            public T call(T acc, T value) {
                if (flag * acc.compareTo(value) > 0) {
                    return acc;
                }
                return value;
            }
        });
    }

flag must be long as Integer.MIN_VALUE == -1 * Integer.MIN_VALUE. But we will have one multiplication for every compareTo.

Another solution is:

    public static <T extends Comparable<T>> Observable<T> min(
            Observable<T> source) {
        return minMax(source, true);
    }

    public static <T extends Comparable<T>> Observable<T> max(
            Observable<T> source) {
        return minMax(source, false);
    }

    public static <T extends Comparable<T>> Observable<T> minMax(
            Observable<T> source, final boolean isMin) {
        return source.reduce(new Func2<T, T, T>() {
            @Override
            public T call(T acc, T value) {
                if (isMin) {
                    if (acc.compareTo(value) < 0) {
                        return acc;
                    }
                } else {
                    if (acc.compareTo(value) > 0) {
                        return acc;
                    }
                }
                return value;
            }
        });
    }

@samuelgruetter , do you have other better solution?

@samuelgruetter
Copy link
Contributor

I like both of these two solutions, with a slight preference for the first one ;-)
Nice catch with Integer.MIN_VALUE :-)

A third solution would be to implement max using min and wrapping the given comparator such that it inverts the ordering, but I think the first solution is the best.

Another issue: What if there are several minimal elements? Does min return the first of them, the last of them, or is it unspecified? This should be documented.

@zsxwing
Copy link
Member Author

zsxwing commented Nov 11, 2013

@samuelgruetter , Thanks for your review. I used the '+1/-1' way to implement it and also updated the document.

@cloudbees-pull-request-builder

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

@benjchristensen
Copy link
Member

Looks good @zsxwing and thanks @samuelgruetter for the review.

Some nice use of generics in those signatures!

benjchristensen added a commit that referenced this pull request Nov 12, 2013
Implemented the "Operator: Min and MinBy" and "Operator: Max and MaxBy"
@benjchristensen benjchristensen merged commit 7475a15 into ReactiveX:master Nov 12, 2013
@zsxwing zsxwing deleted the min-max branch November 13, 2013 02:36
rickbw pushed a commit to rickbw/RxJava that referenced this pull request Jan 9, 2014
Implemented the "Operator: Min and MinBy" and "Operator: Max and MaxBy"
jihoonson pushed a commit to jihoonson/RxJava that referenced this pull request Mar 6, 2020
Add find() method that will only return an existing object in a Registry
but will not create a new one.
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