commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: [CSV] Prohibit creation of more than one iterator over a CSVParser?
Date Wed, 14 Aug 2013 18:22:59 GMT
Thanks for the input. Now is the time to talk about this kind of stuff.
I understand Matt's proposal and it should be relatively easy to implement.

However I see Paul's point but don't know yet how to develop the API the
way he suggests.
Paul can your point be summed up as: Either expose an iterator or a parser
but not both?

What do others think? Sebb? Gary? Emmanuel?
This seems like one of the last issues on the road to 1.0

Benedikt


2013/8/13 Paul Benedict <pbenedict@apache.org>

> Perhaps we're going down the wrong path here. I think having an iterator()
> and a parser conflict with each other. The former seems like a leaky
> abstraction of the latter.  I would propose the parser uses an iterator
> internally, or there's an API to get an iterator from a data source (but
> not expose the parser).
>
> Paul
>
>
> On Tue, Aug 13, 2013 at 9:00 AM, Matt Benson <gudnabrsam@gmail.com> wrote:
>
> > One approach I think I've actually used at some point is for the same
> class
> > to implement both Iterator and Iterable, implementing #iterator() as
> > `return this`.  While it seems a little weird, it's really just a more
> > explicit reflection of what the current code is doing and so, at least in
> > this case, it seems to me justifiable.  WDYT?
> >
> > Matt
> >
> >
> > On Tue, Aug 13, 2013 at 8:39 AM, Benedikt Ritter <britter@apache.org>
> > wrote:
> >
> > > We had that before but switched to Iterable to make it possible to use
> > > CSVPaarser in foreach loops.
> > >
> > >
> > > 2013/8/13 Matt Benson <gudnabrsam@gmail.com>
> > >
> > > > My thinking was more that CSVParser itself implements Iterator.
> > > >
> > > > Matt
> > > > On Aug 13, 2013 2:59 AM, "Benedikt Ritter" <britter@apache.org>
> wrote:
> > > >
> > > > > Hi Matt,
> > > > >
> > > > >
> > > > > 2013/8/12 Matt Benson <gudnabrsam@gmail.com>
> > > > >
> > > > > > As someone with no prior involvement with this component, and
at
> > risk
> > > > of
> > > > > > being hit by the digital tomatoes of the group, this seems to
> > > indicate
> > > > to
> > > > > > me that once a parser definition has been joined to a source
of
> > > input,
> > > > > the
> > > > > > resulting object *is* the record iterator.  If there's no way
to
> > > twist
> > > > > that
> > > > > > into a comfortable API, I would tend to agree with Benedikt:
> >  calling
> > > > > > #iterator() a second time should do something like triggering
an
> > > > > > IllegalStateException().
> > > > > >
> > > > >
> > > > > No tomatoes, don't worry ;-) feedback is always welcome.
> > > > >
> > > > > I'm not sure I understand what you're suggesting. Are you thinking
> of
> > > > > something like:
> > > > >
> > > > > Iterator<CSVRecord> itr = CSVParser.parse(myCsvFile);
> > > > >
> > > > > CSVParser has some features that go beyond the capabilities of the
> > > > > Iterator() interface. For example one can ask the parser for the
> > > current
> > > > > line number in your input and the current record number (which may
> > not
> > > be
> > > > > the same for multi line records).
> > > > > How about extending the Iterator interface?
> > > > >
> > > > > CSVIterator itr = CSVParser.parse(myCsvFile);
> > > > >
> > > > > and CSVIterator would look like:
> > > > >
> > > > > public interface CSVIterator extends Iterator<CSVRecord> {
> > > > >    // call the cool CSVParser stuff goes here
> > > > > }
> > > > >
> > > > > But this wouldn't prevent more than one iterator over the same
> > > source...
> > > > >
> > > > > Benedikt
> > > > >
> > > > >
> > > > > >
> > > > > > $0.02,
> > > > > > Matt
> > > > > >
> > > > > >
> > > > > > On Mon, Aug 12, 2013 at 4:43 PM, Gary Gregory <
> > > garydgregory@gmail.com
> > > > > > >wrote:
> > > > > >
> > > > > > > On Mon, Aug 12, 2013 at 3:26 PM, Benedikt Ritter <
> > > britter@apache.org
> > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > I've added a new test to CSVParser test case that
shows what
> > > > happens
> > > > > if
> > > > > > > > CSVParser.iterator() is called twice [1].
> > > > > > > >
> > > > > > > > This looks pretty strange to me. One iterator can
eat up
> > records
> > > of
> > > > > the
> > > > > > > > other.
> > > > > > > > Would it be better to throw an exception if iterator()
is
> > called
> > > > more
> > > > > > > than
> > > > > > > > once?
> > > > > > > >
> > > > > > >
> > > > > > > Yeah, there is something odd about the current impl. Wouldn't
> it
> > be
> > > > > > obvious
> > > > > > > what can be done if there is an iterator ivar and the accessor
> > just
> > > > > > returns
> > > > > > > it? It does not even have to be lazy initialized.
> > > > > > >
> > > > > > > Gary
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Benedikt
> > > > > > > >
> > > > > > > > [1] http://svn.apache.org/r1513228
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > http://people.apache.org/~britter/
> > > > > > > > http://www.systemoutprintln.de/
> > > > > > > > http://twitter.com/BenediktRitter
> > > > > > > > http://github.com/britter
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > E-Mail: garydgregory@gmail.com | ggregory@apache.org
> > > > > > > Java Persistence with Hibernate, Second Edition<
> > > > > > > http://www.manning.com/bauer3/>
> > > > > > > JUnit in Action, Second Edition <
> > http://www.manning.com/tahchiev/>
> > > > > > > Spring Batch in Action <http://www.manning.com/templier/>
> > > > > > > Blog: http://garygregory.wordpress.com
> > > > > > > Home: http://garygregory.com/
> > > > > > > Tweet! http://twitter.com/GaryGregory
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > http://people.apache.org/~britter/
> > > > > http://www.systemoutprintln.de/
> > > > > http://twitter.com/BenediktRitter
> > > > > http://github.com/britter
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > http://people.apache.org/~britter/
> > > http://www.systemoutprintln.de/
> > > http://twitter.com/BenediktRitter
> > > http://github.com/britter
> > >
> >
>
>
>
> --
> Cheers,
> Paul
>



-- 
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message