Added helper method to get the amount of rows from the Grid

[mstahv]

closes #8679

---

This change is 

[hesara]

Reviewed 1 of 2 files at r1.
Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.

---

server/src/main/java/com/vaadin/ui/Grid.java, line 3419 at r1 (raw file):

    /**
     * @return the amount of rows in this Grid

number of rows
(I would have fixed this myself, but don't seem to have push access to the PR branch)

---

Comments from Reviewable

[mstahv]

Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.

---

server/src/main/java/com/vaadin/ui/Grid.java, line 3419 at r1 (raw file):

Previously, hesara (Henri Sara) wrote…

number of rows
(I would have fixed this myself, but don't seem to have push access to the PR branch)

Don't know why. I'm pretty sure the checkbox was on when I did the PR. I checked that on now (after the PR was done).

---

Comments from Reviewable

[pleku]

Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

a discussion (no related file):
I'm not even sure if we want to add this method, especially like this as it can have some performance indications and requires lot of explaining.

The simplified version could be to cache the latest reported size in data communicator, and then delegate the added size() method to fetch that.

---

Comments from Reviewable

[mstahv]

The need to get the number of rows is so common that the method for sure is needed. I personally don't care at all how it is implemented. I know nothing about Grid's internals (and don't want to know at this point), so I just copied the implementation from the Grid class from an other place where the number of rows was needed. If the size query is slow in the backend, developers probably need to cache that somehow anyways. If this could be optimised at Grids level, it could be done later.

[pleku]

Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

a discussion (no related file):

Previously, pleku (Pekka Hyvönen) wrote…

I'm not even sure if we want to add this method, especially like this as it can have some performance indications and requires lot of explaining.

The simplified version could be to cache the latest reported size in data communicator, and then delegate the added size() method to fetch that.

Yeah my concern is just that with the current implementation, it is not the number of rows in grid, but the number of rows in the used backend. The grid might actually have different amount of rows visible if the data set has changed and grid hasn't been updated. So for the use case, visualize number of rows in grid, this is not a valid solution.

I might push another patch for this that used as cached value from data provider, thus it would be lot simpler.

---

Comments from Reviewable

[mstahv]

See this as well (I don't know how Grid works, but you might want to use the cached value here as well):
https://github.com/mstahv/framework/blob/e86c18bbe155d6bb7381feb6b0873441c44a8a7b/server/src/main/java/com/vaadin/ui/Grid.java#L3397

[pleku]

Thanks, I'll fix that too

[pleku]

Closing this PR because it has multiple issues, described in the ticket #8679, and this approach just won't work.