Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GetAllAsync overload without parameter simplify getting all or by keys #16264

Closed

Conversation

ronaldbarendse
Copy link
Contributor

As mentioned in #16230 (comment), changing GetAllAsync() to return all data types if no keys are specified requires conditional checks when supplying a dynamic list of keys (as an empty list of keys would now return all data types, instead of 0).

To avoid having to manually check the length of the list of keys, this PR adds an overload without the parameter that returns all data types and reverts the previous method to only return data types for the specified keys (so an empty list of keys results in an empty list of data types again).

This should also prevent similar bugs in the future (or in packages). The only discrepancy would be that GetAll(params int[] ids) still returns all data types if an empty list of IDs is specified), but this methods has already been obsoleted.

@ronaldbarendse ronaldbarendse requested a review from kjac May 10, 2024 12:31
@kjac
Copy link
Contributor

kjac commented May 28, 2024

Hi @ronaldbarendse

Thanks for this ❤️ sorry it's taken a while to get back to you.

While this makes a ton of sense, we are too close to V14 to make this change. We also would like to see this change on every service layer that exhibits this behavior.

In short: We are going to move this task to the backlog. In the future we'll introduce the following methods on the relevant service layers:

  • GetAllAsync: Has no parameters, always returns all items.
  • GetManyAsync: Accepts a params parameter of keys, returns only the requested items - and none, if the params parameter is empty.

@kjac kjac closed this May 28, 2024
@ronaldbarendse ronaldbarendse deleted the v14/feature/datatypeservice-getallasync branch May 28, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants