Skip to content

Conversation

@rstam
Copy link
Contributor

@rstam rstam commented Nov 15, 2025

No description provided.

@rstam rstam requested a review from a team as a code owner November 15, 2025 17:01
@rstam rstam added the improvement Optimizations or refactoring (no new features or fixes). label Nov 15, 2025
sanych-sun
sanych-sun previously approved these changes Nov 17, 2025
Copy link
Member

@sanych-sun sanych-sun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we are here, could we also remove the unnecessary else blocks in some of the methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I noticed that the methods: TryGetIEnumerableGenericInterface; TryGetIListGenericInterface; TryGetIQueryableGenericInterface all have nearly identical implementations. Could they be consolidated to use the existing TryGetGenericInterface helper?

something like:

  public static bool TryGetIEnumerableGenericInterface(this Type type, out Type ienumerableGenericInterface)
  {
      return TryGetGenericInterface(type, new[] { typeof(IEnumerable<>) }, out ienumerableGenericInterface);
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done except slightly differently. I added an overload of TryGetGenerricInterface that only takes a single generic interface definition to match against. Should be slightly more efficient than the array version when there is only one definition involved.

sanych-sun
sanych-sun previously approved these changes Nov 17, 2025
Copy link
Member

@sanych-sun sanych-sun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

adelinowona
adelinowona previously approved these changes Nov 18, 2025
Copy link
Contributor

@adelinowona adelinowona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Optimizations or refactoring (no new features or fixes).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants