Refactor Min/Max methods with better selector and comparer overload support

[mobilebilly]

Refactor Min/Max Methods to Support Generic Selector Overloads and Separate Comparer Variants

This pull request refactors the Min and Max methods to provide greater flexibility through generic selector overloads, and separates the comparer-based logic into clearly named methods.

Changes

• Added generic overloads:

  public Min<R>(selector?: (element: T) => R): R | undefined  
  public Max<R>(selector?: (element: T) => R): R | undefined
  

• Introduced comparer-specific variants:

  public MinWithComparer(comparer?: (element: T) => number): T | undefined  
  public MaxWithComparer(comparer?: (element: T) => number): T | undefined
  

[coveralls]

coverage: 100.0%. remained the same
when pulling ad5ea0d on mobilebilly:feature/max-min-overloads
into 83b9adb on kutyel:master.

[kutyel]

Is this another breaking change? 🤔 I can't seem to find this *WithComparer functions in the C# library dotnet/runtime#18481 👀

[mobilebilly]

Thanks for asking. Yes, this is a breaking change—the return type of Max/Min has changed from T to R, where R is the type returned by the selector function, Max/Min replicate the behavior of the Max/Min overload wtih selector argument.

The WithComparer variants are introduced to replicate the overload behavior of Max/Min overload with Comparer argument.

However, due to TypeScript's lack of true function overloading, I had to use different function names.

[mobilebilly]

I will take further look the coming days to make further reviews and improvements, no hurry.

[kutyel]

I'd like to know the opinion of @typescriptbob on this (if he is still around) as another data point since he is also a C# fellow (AFAIK) 😉

[mobilebilly]

Hi @kutyel,

I have removed the *WithComparer Variants and re-implemented the Min and Max functions with function overloading.

Max function overloads

  public Max(): T | undefined;
  public Max<R>(selector: (e: T) => R): R | undefined;
  public Max(comparer: (a: T, b: T) => number): T | undefined;

Max function implementation

public Max<R>(
    comparerOrSelector?: ((a: T, b: T) => number) | ((e: T) => R)
  ): T | R | undefined 

Min function overloads

  public Min(): T | undefined;
  public Min<R>(selector: (e: T) => R): R | undefined;
  public Min(comparer: (a: T, b: T) => number): T | undefined;

Min function implementation

public Min<R>(
    comparerOrSelector?: ((a: T, b: T) => number) | ((e: T) => R)
  ): T | R | undefined 

In the implementation, the length of the comparerOrSelector function is inspected to determine the type of the function delegate received, length=1 if it is a selector, length=2 if it is a comparer.

Another enhancement is to use elements.reduce instead of a custom foreach loop to get the max/min values

[kutyel]

In the implementation, the length of the comparerOrSelector function is inspected to determine the type of the function delegate received, length=1 if it is a selector, length=2 if it is a comparer.

Another enhancement is to use elements.reduce instead of a custom foreach loop to get the max/min values

Awesome! I saw you were able to use TypeScript's overload syntax to make it work, nice! 👏🏻

However, I'm a bit concerned about the usage of @ts-ignore in the codebase, it is generally not a good sign, what is the exact type error?

[mobilebilly]

Earlier today, there was a build error that I temporarily bypassed using @ts-ignore in the test case.
However, I can no longer reproduce the issue, so I’ve removed the @ts-ignore annotation.

[github-actions]

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀