I think I'm actually with Romain here and I'm somewhat in favor of dropping this. 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> wrote: > 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> >>>>> >>>> >>> >
