feat(maxBy): Add maxBy function that select element that have max value by given condition in array

[l2hyunwoo]

close #55

I added a function that select element that have max value by given condition in array.

• If input array is not empty, return first element of array that have maximum value that satisfies given condition
• If input array is empty, return undefined

[vercel]

@l2hyunwoo is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

[raon0211]

Suggested change

	export function maxBy<T>(elements: T[], selector: (element: T) => number): T | undefined {
	export function maxBy<T>(elements: T[], selector: (element: T) => number): T {

I believe returning undefined is stricter and less error-prone.

However, following TypeScript's convention that arr[0] is not nullable even if arr is empty (unless noUncheckedIndexedAccess is on) is also important for consistency -- that's why we chose sample to not return nullable even if it can return undefined.

For the sake of consistency, I believe we should not return undefined here.

What do you think?

[l2hyunwoo]

However, following TypeScript's convention that arr[0] is not nullable even if arr is empty

I tested this on TS Playground. And the result is below.

const array: number[] = [];
console.log(array[0]);

// [LOG]: undefined 

I think this log says it's null(or undefined) when point first element of empty array.

---

I think usage without undefined makes compilcated usecase. Let's see below example code.

When we use undefined

  const maxElement = maxBy<number>([]) ?? 0;

When we use Exception

let maxElement;
try {
  maxElement = maxBy<number>([]);
} catch (error) {
  // TODO: Logic when error occurred
}

const result = maxElement;

I think above one is more simple. What do you think?

---

But for the sake of consistency in this library, I also think it should not return undefined. Thanks for your advice.

[raon0211]

Oh, what I thought was that here item is not nullable (in static types) even if array is empty.

[raon0211]

So I thought dealing with empty arrays is what users of es-toolkit have to handle in runtime...
(We do not check that the array is not empty using types)

[l2hyunwoo]

I see... It seems that usual convention in typescript. It's so hard to admit, but now I'm understand what you wrote. I'll change to non-nil type! Thanks for check it.

[l2hyunwoo]

After modification, all test cases pass well 👍🏻

[raon0211]

Suggested change

	if (elements.length === 0) {
	return undefined;
	}
	let maxElement = elements[0];
	let max = selector(maxElement);
	for (const element of elements) {
	const value = selector(element);
	if (value > max) {
	max = value;
	maxElement = element;
	}
	}
	return maxElement;
	let maxElement = elements[0];
	let max = -Infinity;
	for (const element of elements) {
	const value = selector(element);
	if (value > max) {
	max = value;
	maxElement = element;
	}
	}
	return maxElement;

[l2hyunwoo]

It's more readable I think. I'll fix it to this.

[l2hyunwoo]

It changed on this commit

[raon0211]

Thanks, @l2hyunwoo !

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (3ecc062) to head (e4f8167).
Report is 382 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #64   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           49        50    +1     
  Lines          244       253    +9     
  Branches        27        28    +1     
=========================================
+ Hits           244       253    +9