Parameterized Sort #430
Description
The ability to parameterize Sort objects was proposed by @gavinking while reviewing #394 (static metamodel). I'm opening this issue to give parameterized Sort its own discussion.
Here are are the scenarios that were brought up:
List<Customer> repository.allCustomers(_Customer.name.asc())
Employee[] findByYearHired(int yearYired, Limit maxResults, Sort<Employee>... sortBy);
In both of these cases, the type parameter helps to ensure that Sort for the correct type of entity must be supplied. It helps to enforce correctness at development time, which is nice.
One problem is that I see a warning for using variable arguments ... on a parameterized type (the second example with Sort<Employee>... sortBy): Type safety: Potential heap pollution via varargs parameter sortBy
You can suppress that warning with @SuppressWarnings("unchecked"), but you also get a warning on usage: Type safety: A generic array of Sort<Employee> is created for a varargs parameter, requiring a @SuppressWarnings("unchecked") everywhere you use it.
Switching to Sort<?>... sortBy avoids the warning but that also bypasses being type safe.
Switching to Sort<Employee>[] works without warnings on the interface declaration,
Employee[] findByYearHired(int yearYired, Limit maxResults, Sort<Employee>[] sortBy);
but unfortunately gets an error trying to use it,
Employee[] e = employees.findByYearHired(2024, limit, new Sort<Employee>[] { mySort });
Cannot create a generic array of Sort<Employee>.
Unless someone has other ideas here, I think we would need to switch to allow List<Sort<Employee>> as a repository method parameter in order to get around these issues.
Another aspect of this is that Pageable (which can have a varags array of Sort) must also be given a type-parameter.
We can do that, but then usage needs to switch from,
Pageable<Employee> pageRequest = Pageable.ofSize(10).sortBy(mySort1, mySort2);
to either
Pageable<Employee> pageRequest = Pageable.<Employee> ofSize(10).sortBy(List.of(sort1, sort2));
or
Pageable<Employee> pageRequest = Pageable.ofSize(Employee.class, 10).sortBy(List.of(sort1, sort2));
That is certainly doable, but we will need to think about whether the additional type safety is worth the trade-off. I don't have an answer of my own at this point - I'm still exploring the scenarios.
See pull #432 to help evaluate what this change would do to the API/usage.