Clarified documentation about Iterable data providers and size() calls

[AndreasTu]

Added note to the documentation to clarify the usage of Iterables
as data pipes and the used size() method.

Fixes #2022

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.86%. Comparing base (2c7db77) to head (e026689).
Report is 157 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2027      +/-   ##
============================================
+ Coverage     80.44%   81.86%   +1.41%     
- Complexity     4337     4609     +272     
============================================
  Files           441      448       +7     
  Lines         13534    14445     +911     
  Branches       1707     1829     +122     
============================================
+ Hits          10888    11826     +938     
+ Misses         2008     1942      -66     
- Partials        638      677      +39     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Vampire]

I don't think we should do this or anything similar.
If it is a Collection it could also be expensive to calculate.
For Iterator it just is not done as Iterators are one-time consumable only.
For all others you can consume it multiple times and users should adjust to that imho.
They can anytime give an interator in instead, or make sure the iterable has some built-in cache to not do the expensive operation multiple times.

[AndreasTu]

@Vampire But why should we call a size() method on an Iterable, which is not part of the contract for Iterable.
On Collections there is the size() method contract, which we can argument "hey then apply a cache", but the call to DefaultGroovyMethods.size(Iterable) is IMHO very surprising, which leads to 3 iterations of the iterable, where the user did not define it.

I think the size() method on an iterable is a Groovy idiosyncrasy, which we should not force onto users.

[Vampire]

But why should we call a size() method on an Iterable, which is not part of the contract for Iterable.

Spock specs are Groovy code, not Java code.
In Groovy size() is part of the contract of Iterable: https://docs.groovy-lang.org/latest/html/groovy-jdk/java/lang/Iterable.html#size()

I think the size() method on an iterable is a Groovy idiosyncrasy, which we should not force onto users.

I'm totally on the opposite actually.
We are in Groovy environment, so we should follow Groovy semantics.
And for Groovy the method is defined and thus executed.

@leonard84 what is your PoV?

[aditbhartia]

would specifying that iteration over the Iterable may be done multiple times before execution be a more accurate way of expressing the internal behavior? In my opinion, a reader could assume that the iteration is only done once from reading this. The implication that the next value is fetched unnecessarily also seems to contradict the statement in 253.

[Vampire]

would specifying that iteration over the Iterable may be done multiple times before execution be a more accurate way of expressing the internal behavior?

Maybe in some cases, but not generally.
Also the user documentation should not document the internal behavior.
And actually, it is not even Spock behavior.
Spock just calls the size() method whatever that may be.
It is Groovy that - if no other size() method is present - for an Iterable calls iterator() and iterates over it to determine the size().
And this could also any time change in any Groovy version theoretically.
So this should imho not be documented in the Spock doc.

In my opinion, a reader could assume that the iteration is only done once from reading this. The implication that the next value is fetched unnecessarily also seems to contradict the statement in 253.

Yes and no.
We indeed only fetch the next value when it is needed as documented.
As said above, it is the Groovy implementation of size() that iterates over an Iterable to determine its size.
All we can say imho is, that we call size() if it is not an Iterator and that it should be efficient.
Then the user can take care to make it efficient or if not possible provide an Iterator.

In the case of an external DB, you could e. g. also provide an Iterable that has a size() method that on first call does a select count query and persist that result or similar.

[Vampire]

would specifying that iteration over the Iterable may be done multiple times before execution be a more accurate way of expressing the internal behavior?

Maybe in some cases, but not generally.
Also the user documentation should not document the internal behavior.
And actually, it is not even Spock behavior.
Spock just calls the size() method whatever that may be.
It is Groovy that - if no other size() method is present - for an Iterable calls iterator() and iterates over it to determine the size().
And this could also any time change in any Groovy version theoretically.
So this should imho not be documented in the Spock doc.

In my opinion, a reader could assume that the iteration is only done once from reading this. The implication that the next value is fetched unnecessarily also seems to contradict the statement in 253.

Yes and no.
We indeed only fetch the next value when it is needed as documented.
As said above, it is the Groovy implementation of size() that iterates over an Iterable to determine its size.
All we can say imho is, that we call size() if it is not an Iterator and that it should be efficient.
Then the user can take care to make it efficient or if not possible provide an Iterator.

In the case of an external DB, you could e. g. also provide an Iterable that has a size() method that on first call does a select count query and persist that result or similar.

[Vampire]

lgtm, thx