flow
flow copied to clipboard
Use of long from DataProvider.size
DataProvider.size method returns an int and I find myself to always cast longs from repositories, e.g. JPA CriteriaBuilder.count or Spring's CrudRepository.count which both returns longs.
Why not making DataProvider.size to return a long?
Some considerations of varying weight:
- If we change type of the size method, then we should for consistency also change the type of the offset and limit in the query. This in turn would require application code to downcast those to
intwith other APIs that don't acceptlongwhen querying. - Changing it at this point would be backwards incompatible in a way that affects very many applications.
- Returning
longwould give the impression that it's a good idea to have a grid with more than 2000000 rows, whereas that is most likely quite problematic from a usability point of view.
Thank you @Legioth for your considerations.
I'm pointing this out for two reasons mainly:
- since the Data Provider API requires a
Streamfromfetch, andStreamreturns/acceptslongforcount,skipandlimit, I'd expect consistency with the Java Stream API, i.e.
DataProvider.fromCallbacks(
query -> getStream().skip(query.getOffset()).limit(query.getLimit()),
query -> getStream().count()
);
but instead I need to cast the count to int, or use Math.toIntExact.
- you are concerned about giving the impression that isn't a good idea to have a grid with that many rows, but I don't really get why: if the user has a way to filter data, why limiting the backend domain for
DataProviders to integer values? It's quite limiting in my opinion, I sure have more than one use cases where I'll hit the integer limit and this would cause trouble.
From Slack discussion by @Artur- :
The problem is not that you want to show ”1 item out of 2647588583617388577448 matched”, the problem is if your app stops working when it has been running in production for a long time
Returning long would give the impression that it's a good idea to have a grid with more than 2000000 rows, whereas that is most likely quite problematic from a usability point of view.
That is not up to Grid to decide. If you have a huge table, then that's it - hardly you'll artificially delete rows to make Grid happy. I agree that scrolling through such a table doesn't probably make much sense from the UX perspective. Yet showing this kind of table in a Grid is really tricky: how do you convert a large long to int?
- You cast to
int, risking arithmetic overflow and possibly telling the customer that the table has zero rows when it hasInt.MAX+1 - You use
Math.toIntExactand your code throws an exception - You cap the count at
Int.MAX- probably the best option, but then if something else than Grid depends on the outcome ofDataProvider.count(), that code will show inaccurate result. - You have to do this kind of decision every time you work with
DataProvider.
This case is probably quite rare - I personally haven't seen Grid attempting to show so many rows, so this is a corner case for sure - but when you hit this corner-case, it's not clear what to do. The best solution is to make DataProvider work with longs instead of ints, but I agree this is a breaking change.
The best solution is to make DataProvider work with longs instead of ints, but I agree this is a breaking change.
Workaround might be as simple as grid.getLazyDataView().setItemCountUnknown() - no count call, no problem ;)