I think I'm actually with Romain here and I'm somewhat in favor of dropping
The only entity who can violate the start/advance/getCurrent contract is
the runner or a test utility, and they have their own unit tests to ensure
they don't - there is no reason to re-enforce their contract via every IO
that uses the Source API. It's a minor point, but it does clutter the code
of every such IO a bit.

On Mon, Mar 12, 2018 at 9:40 AM Romain Manni-Bucau <rmannibu...@gmail.com>

> Hmm, wrong link? Iterator contract doesn't require hasNext() to be called
> compared to beam (even if it is highly recommanded). Aligned on beam it
> means the runner can acll getCurrent() without calling start or advanced
> and consider it is done if there is a NSEE which will likely never works
> since start/advance bufferizes current in most (all?) impl. So it is likely
> a bit different.
> To be concrete iterator often handle hasNext implicitly in next() if
> needed whereas beam IOs never do it.
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
> 2018-03-12 17:35 GMT+01:00 Thomas Groh <tg...@google.com>:
>> The correct sequencing and respecting return values of `start` and
>> `advance` is a precondition, and `NoSuchElementException` is the failure
>> mode if the precondition isn't met. Documenting the behavior in the case of
>> precondition failures is entirely reasonable. For example, look at Java's
>> `Iterator` documentation, which has basically the same text:
>> https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html
>> On Mon, Mar 12, 2018 at 9:20 AM Romain Manni-Bucau <rmannibu...@gmail.com>
>> wrote:
>>> I agree Thomas but I kind of read it as "yes we can drop that
>>> constraint". If not we should also check we are used in a thread safe
>>> context etc which will likely never hit the user sdk API so why doing that
>>> case a particular case? Am I missing something?
>>> Romain Manni-Bucau
>>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>> <http://rmannibucau.wordpress.com> | Github
>>> <https://github.com/rmannibucau> | LinkedIn
>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>
>>> 2018-03-12 17:04 GMT+01:00 Thomas Groh <tg...@google.com>:
>>>> If a call to `getCurrentWhatever` happens after `start` or `advance`
>>>> has returned false, it's a bug in the runner, but the reader needs to be
>>>> able to fail, otherwise you'll get a synthetic element that doesn't really
>>>> exist. If a reader throws `NoSuchElementException` after the most recent
>>>> call returned true, the reader isn't conforming to spec.
>>>> On Mon, Mar 12, 2018 at 9:00 AM Romain Manni-Bucau <
>>>> rmannibu...@gmail.com> wrote:
>>>>> Hi guys,
>>>>> why reader#getCurrent* can throw NoSuchElementException,
>>>>> my understanding is that the runner will guarantee that start or
>>>>> advance was called and returned true when calling getCurrent so this is a
>>>>> case which shouldn't happen, no?
>>>>> Romain Manni-Bucau
>>>>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>>>>> <https://rmannibucau.metawerx.net/> | Old Blog
>>>>> <http://rmannibucau.wordpress.com> | Github
>>>>> <https://github.com/rmannibucau> | LinkedIn
>>>>> <https://www.linkedin.com/in/rmannibucau> | Book
>>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance>

Reply via email to