From 37c568af5952a0bc7f9c9305be267a3a143af37e Mon Sep 17 00:00:00 2001 From: abrwn Date: Tue, 22 Feb 2022 23:27:30 +0000 Subject: [PATCH] Improve min and max logic (#3197) * Improve min and max logic * Fix import and update tests * PR comments * Change const to var, calculate toString once Co-authored-by: Alex Browne --- source/max.js | 23 +++++++++++++++++++++-- source/maxBy.js | 6 ++++-- source/min.js | 23 +++++++++++++++++++++-- source/minBy.js | 5 +++-- test/max.js | 24 ++++++++++++++++++++++++ test/min.js | 24 ++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 8 deletions(-) diff --git a/source/max.js b/source/max.js index f46f6ba621..28c76f5654 100644 --- a/source/max.js +++ b/source/max.js @@ -1,5 +1,5 @@ import _curry2 from './internal/_curry2.js'; - +import toString from './toString.js'; /** * Returns the larger of its two arguments. @@ -18,5 +18,24 @@ import _curry2 from './internal/_curry2.js'; * R.max(789, 123); //=> 789 * R.max('a', 'b'); //=> 'b' */ -var max = _curry2(function max(a, b) { return b > a ? b : a; }); +var max = _curry2(function max(a, b) { + if (a === b) { return b; } + + function safeMax(x, y) { + if ((x > y) !== (y > x)) { return y > x ? y : x; } + return undefined; + } + + var maxByValue = safeMax(a, b); + if (maxByValue !== undefined) { return maxByValue; } + + var maxByType = safeMax(typeof a, typeof b); + if (maxByType !== undefined) { return maxByType === typeof a ? a : b; } + + var stringA = toString(a); + var maxByStringValue = safeMax(stringA, toString(b)); + if (maxByStringValue !== undefined) { return maxByStringValue === stringA ? a : b; } + + return b; +}); export default max; diff --git a/source/maxBy.js b/source/maxBy.js index 936f5c4420..fddaff4817 100644 --- a/source/maxBy.js +++ b/source/maxBy.js @@ -1,5 +1,5 @@ import _curry3 from './internal/_curry3.js'; - +import max from './max.js'; /** * Takes a function and two values, and returns whichever value produces the @@ -26,6 +26,8 @@ import _curry3 from './internal/_curry3.js'; * R.reduce(R.maxBy(square), 0, []); //=> 0 */ var maxBy = _curry3(function maxBy(f, a, b) { - return f(b) > f(a) ? b : a; + var resultB = f(b); + return max(f(a), resultB) === resultB ? b : a; }); + export default maxBy; diff --git a/source/min.js b/source/min.js index d7dd885344..fad0def9ac 100644 --- a/source/min.js +++ b/source/min.js @@ -1,5 +1,5 @@ import _curry2 from './internal/_curry2.js'; - +import toString from './toString.js'; /** * Returns the smaller of its two arguments. @@ -18,5 +18,24 @@ import _curry2 from './internal/_curry2.js'; * R.min(789, 123); //=> 123 * R.min('a', 'b'); //=> 'a' */ -var min = _curry2(function min(a, b) { return b < a ? b : a; }); +var min = _curry2(function min(a, b) { + if (a === b) { return a; } + + function safeMin(x, y) { + if ((x < y) !== (y < x)) { return y < x ? y : x; } + return undefined; + } + + var minByValue = safeMin(a, b); + if (minByValue !== undefined) { return minByValue; } + + var minByType = safeMin(typeof a, typeof b); + if (minByType !== undefined) { return minByType === typeof a ? a : b; } + + var stringA = toString(a); + var minByStringValue = safeMin(stringA, toString(b)); + if (minByStringValue !== undefined) { return minByStringValue === stringA ? a : b; } + + return a; +}); export default min; diff --git a/source/minBy.js b/source/minBy.js index aa66015aca..787875fa26 100644 --- a/source/minBy.js +++ b/source/minBy.js @@ -1,5 +1,5 @@ import _curry3 from './internal/_curry3.js'; - +import min from './min.js'; /** * Takes a function and two values, and returns whichever value produces the @@ -26,6 +26,7 @@ import _curry3 from './internal/_curry3.js'; * R.reduce(R.minBy(square), Infinity, []); //=> Infinity */ var minBy = _curry3(function minBy(f, a, b) { - return f(b) < f(a) ? b : a; + var resultB = f(b); + return min(f(a), resultB) === resultB ? b : a; }); export default minBy; diff --git a/test/max.js b/test/max.js index 692208d2ef..7534aa42cc 100644 --- a/test/max.js +++ b/test/max.js @@ -19,4 +19,28 @@ describe('max', function() { eq(R.max('b', 'a'), 'b'); }); + it('returns the second argument if both arguments are equal according to the native JS strict equals operator', function() { + eq(R.max(7, 7), 7); + eq(R.max(undefined, undefined), undefined); + }); + + it('returns the alphabetically later type if neither argument is greater than the other', function() { + eq(R.max('a', 7), 'a'); + eq(R.max('a', undefined), undefined); + }); + + it('returns the alphabetically later string coersion if no argument or type is greater than the other', function() { + const obj1 = { a: 1 }; + const obj2 = { b: 1 }; + + eq(R.max(obj1, obj2), obj2); + eq(R.max(obj1, null), obj1); + }); + + it('returns the first argument if no other comparison attempts produce a result', function() { + const obj1 = { a: 1 }; + + eq(R.max(obj1, obj1), obj1); + eq(R.max(NaN, NaN), NaN); + }); }); diff --git a/test/min.js b/test/min.js index 61f4edd7d2..c74a6dca67 100644 --- a/test/min.js +++ b/test/min.js @@ -19,4 +19,28 @@ describe('min', function() { eq(R.min('b', 'a'), 'a'); }); + it('returns the first argument if both arguments are equal according to the native JS strict equals operator', function() { + eq(R.min(7, 7), 7); + eq(R.min(undefined, undefined), undefined); + }); + + it('returns the alphabetically earlier type if neither argument is smaller than the other', function() { + eq(R.min('a', 7), 7); + eq(R.min('a', undefined), 'a'); + }); + + it('returns the alphabetically earlier string coersion if no argument or type is smaller than the other', function() { + const obj1 = { a: 1 }; + const obj2 = { b: 1 }; + + eq(R.min(obj1, obj2), obj1); + eq(R.min(obj1, null), null); + }); + + it('returns the first argument if no other comparison attempts produce a result', function() { + const obj1 = { a: 1 }; + + eq(R.min(obj1, obj1), obj1); + eq(R.min(NaN, NaN), NaN); + }); });