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

Standarise The Parameter Order In the Vault Queries #6904

Open
opticyclic opened this issue May 7, 2021 · 1 comment
Open

Standarise The Parameter Order In the Vault Queries #6904

opticyclic opened this issue May 7, 2021 · 1 comment
Labels

Comments

@opticyclic
Copy link
Contributor

It is very confusing changing from one vault query to another due to the differing order of parameters.

If I want to do a simple query by state, the Contract State Type is the first (and only) parameter:

fun <T : ContractState> vaultQuery(contractStateType: Class<out T>): Vault.Page<T>

If I want to do a more complex query, the Contract State Type is now the last parameter:

fun <T : ContractState> vaultQueryByCriteria(criteria: QueryCriteria, contractStateType: Class<out T>): Vault.Page<T>

If I want to do paging, the Contract State Type and QueryCriteria have switched order and the Contract State Type is now the first parameter again:

fun <T : ContractState> vaultQueryByWithPagingSpec(contractStateType: Class<out T>, criteria: QueryCriteria, paging: PageSpecification): Vault.Page<T>

Similarly with sorting:

fun <T : ContractState> vaultQueryByWithSorting(contractStateType: Class<out T>, criteria: QueryCriteria, sorting: Sort): Vault.Page<T>

The most annoying and confusing part about this is when you are progressing through these as your needs get more complex.

  • State
  • State and Criteria
  • State, Criteria and Paging
  • State, Criteria, Paging and Sorting

Which looks like:

fun <T : ContractState> vaultQuery(contractStateType: Class<out T>): Vault.Page<T>
fun <T : ContractState> vaultQueryByCriteria(criteria: QueryCriteria, contractStateType: Class<out T>): Vault.Page<T>
fun <T : ContractState> vaultQueryByWithPagingSpec(contractStateType: Class<out T>, criteria: QueryCriteria, paging: PageSpecification): Vault.Page<T>
fun <T : ContractState> vaultQueryBy(criteria: QueryCriteria, paging: PageSpecification, sorting: Sort, contractStateType: Class<out T>): Vault.Page<T>

The Contract State Type switches back and forth 4 times!

  • First
  • Last
  • First
  • Last

It makes much more sense to gradually add extra parameters on the end as your needs get more complex.

e.g.

fun <T : ContractState> vaultQuery(contractStateType: Class<out T>): Vault.Page<T>
fun <T : ContractState> vaultQueryByCriteria(contractStateType: Class<out T>, criteria: QueryCriteria): Vault.Page<T>
fun <T : ContractState> vaultQueryByWithPagingSpec(contractStateType: Class<out T>, criteria: QueryCriteria, paging: PageSpecification): Vault.Page<T>
fun <T : ContractState> vaultQueryBy(contractStateType: Class<out T>, criteria: QueryCriteria, paging: PageSpecification, sorting: Sort): Vault.Page<T>

Obviously people are already working with the current design so you would probably have to overload it and maybe mark the older ones as deprecated.

You can also see a gradual logical increase in the names of these functions - until you reach the last use case where it doesn't follow the pattern.
It would make the API a lot easier to discover if the names followed a logical pattern.
Everything delegates to vaultQueryBy in the end under the hood, however, all the others are helpers anyway so there isn't any technical reason why you can't have the final helper just delegate all of its parameters to vaultQueryBy.

It might look a bit strange in the code, however, from an end user point of view, it makes the API a lot easier to understand and discover.

@sachingokhale
Copy link

Ticket for internal review - CORDA-4183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants