Skip to content

Commit

Permalink
fix(min/max): do not return comparer values (ReactiveX#1892)
Browse files Browse the repository at this point in the history
min (&max) operators have now the expected behavior when used with a comparer:
- previously they were behaving like _reduce_ and emitting (last) comparer return value
- they now emit original observable min/max value by comparing the comparer return value to 0
  • Loading branch information
gluck committed Sep 6, 2016
1 parent a61c8b2 commit d9b7a12
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 33 deletions.
24 changes: 12 additions & 12 deletions spec/operators/max-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,42 +184,42 @@ describe('Observable.prototype.max', () => {
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should handle a constant predicate on observable with many values', () => {
const e1 = hot('-x-^-a-b-c-d-e-f-g-|');
const e1subs = '^ !';
const expected = '----------------(w|)';
it('should handle a predicate on observable with many values', () => {
const e1 = hot('-a-^-b--c--d-|', { a: 42, b: -1, c: 0, d: 666 });
const e1subs = '^ !';
const expected = '----------(w|)';

const predicate = () => {
return 42;
const predicate = function (x, y) {
return x > y ? 1 : -1;
};

expectObservable((<any>e1).max(predicate)).toBe(expected, { w: 42 });
expectObservable((<any>e1).max(predicate)).toBe(expected, { w: 666 });
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should handle a predicate on observable with many values', () => {
it('should handle a reverse predicate on observable with many values', () => {
const e1 = hot('-a-^-b--c--d-|', { a: 42, b: -1, c: 0, d: 666 });
const e1subs = '^ !';
const expected = '----------(w|)';

const predicate = function (x, y) {
return Math.min(x, y);
return x > y ? -1 : 1;
};

expectObservable((<any>e1).max(predicate)).toBe(expected, { w: -1 });
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should handle a predicate for string on observable with many values', () => {
const e1 = hot('-1-^-2--3--4-|');
const e1 = hot('-a-^-b--c--d-|');
const e1subs = '^ !';
const expected = '----------(w|)';

const predicate = function (x, y) {
return x > y ? x : y;
return x > y ? -1 : 1;
};

expectObservable((<any>e1).max(predicate)).toBe(expected, { w: '4' });
expectObservable((<any>e1).max(predicate)).toBe(expected, { w: 'b' });
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

Expand Down
28 changes: 14 additions & 14 deletions spec/operators/min-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ describe('Observable.prototype.min', () => {
return 42;
};

expectObservable((<any>e1).min(predicate), unsub).toBe(expected, { w: 42 });
expectObservable((<any>e1).min(predicate), unsub).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

Expand All @@ -188,46 +188,46 @@ describe('Observable.prototype.min', () => {
.min(predicate)
.mergeMap((x: number) => Observable.of(x));

expectObservable(result, unsub).toBe(expected, { w: 42 });
expectObservable(result, unsub).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should handle a constant predicate on observable with many values', () => {
const e1 = hot('-x-^-a-b-c-d-e-f-g-|');
const e1subs = '^ !';
const expected = '----------------(w|)';
it('should handle a predicate on observable with many values', () => {
const e1 = hot('-a-^-b--c--d-|', { a: 42, b: -1, c: 0, d: 666 });
const e1subs = '^ !';
const expected = '----------(w|)';

const predicate = () => {
return 42;
const predicate = (x, y) => {
return x > y ? 1 : -1;
};

expectObservable((<any>e1).min(predicate)).toBe(expected, { w: 42 });
expectObservable((<any>e1).min(predicate)).toBe(expected, { w: -1 });
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should handle a predicate on observable with many values', () => {
it('should handle a reverse predicate on observable with many values', () => {
const e1 = hot('-a-^-b--c--d-|', { a: 42, b: -1, c: 0, d: 666 });
const e1subs = '^ !';
const expected = '----------(w|)';

const predicate = function (x, y) {
return Math.max(x, y);
return x > y ? -1 : 1;
};

expectObservable((<any>e1).min(predicate)).toBe(expected, { w: 666 });
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should handle a predicate for string on observable with many values', () => {
const e1 = hot('-1-^-2--3--4-|');
const e1 = hot('-a-^-b--c--d-|');
const e1subs = '^ !';
const expected = '----------(w|)';

const predicate = function (x, y) {
return x < y ? x : y;
return x > y ? -1 : 1;
};

expectObservable((<any>e1).min(predicate)).toBe(expected, { w: '2' });
expectObservable((<any>e1).min(predicate)).toBe(expected, { w: 'd' });
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

Expand Down
6 changes: 3 additions & 3 deletions src/operator/max.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import { ReduceOperator } from './reduce';
* @method max
* @owner Observable
*/
export function max<T>(comparer?: (x: T, y: T) => T): Observable<T> {
const max: typeof comparer = (typeof comparer === 'function')
? comparer
export function max<T>(comparer?: (x: T, y: T) => number): Observable<T> {
const max: (x: T, y: T) => T = (typeof comparer === 'function')
? (x, y) => comparer(x, y) > 0 ? x : y
: (x, y) => x > y ? x : y;
return this.lift(new ReduceOperator(max));
}
Expand Down
8 changes: 4 additions & 4 deletions src/operator/min.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import { ReduceOperator } from './reduce';
* @method min
* @owner Observable
*/
export function min<T>(comparer?: (x: T, y: T) => T): Observable<T> {
const min: typeof comparer = (typeof comparer === 'function')
? comparer
export function min<T>(comparer?: (x: T, y: T) => number): Observable<T> {
const min: (x: T, y: T) => T = (typeof comparer === 'function')
? (x, y) => comparer(x, y) < 0 ? x : y
: (x, y) => x < y ? x : y;
return this.lift(new ReduceOperator(min));
}

export interface MinSignature<T> {
(comparer?: (x: T, y: T) => T): Observable<T>;
(comparer?: (x: T, y: T) => number): Observable<T>;
}

0 comments on commit d9b7a12

Please sign in to comment.