I would rather avoid implementing List .. we should implement Iterable for sure, but I'd like to keep the API open either iterating in-order or getting a particular SegmentInfo. Another thing, I haven't seen anywhere that remove is called. In general I don't like to impl an interface just to throw UOE everywhere ...
I will open an issue. I usually investigate the code first before I open an issue. Also, what about back-compat? Are we even allowed to change that class? If not, then we can deprecate it and introduce a new one ... Shai On Sun, Feb 28, 2010 at 2:25 PM, Uwe Schindler <u...@thetaphi.de> wrote: > I think you should open an issue! I like this refactoring, maybe we can > still let it implement List<SegmentInfo> but only deprecated and most > methods should throw UOE. Just keep get() and so on. > > > > ----- > > Uwe Schindler > > H.-H.-Meier-Allee 63, D-28213 Bremen > > http://www.thetaphi.de > > eMail: u...@thetaphi.de > > > > *From:* Shai Erera [mailto:ser...@gmail.com] > *Sent:* Sunday, February 28, 2010 1:20 PM > > *To:* java-dev@lucene.apache.org > *Subject:* Re: SegmentInfos extends Vector > > > > Yes that's what I've been thinking as well - SegmentInfos should have a > segments-related API, not a List related. Whether the infos inside are kept > in a Map, List, Collection or array is an implementation detail. In fact, I > have a code which uses the API and could really benefit from a Map-like > interface, but perhaps other code needs things ordered (which is why we can > keep a TreeMap inside, or LinkedHahsMap). That's a great example to why it > should have its own API. > > The Lucene code usually calls SegmentInfos.info(int), but some places call > get(int) (which is inherited from Vector). That's bad. > > SegmentInfos is public, though it's tagged with @lucene.experimental. I > think it should be tagged with @lucene.internal as there's nothing > experimental about it? > > I don't mind doing the refactoring. Not sure how this will affect > back-compat (is it acceptable for this classs?). I've touched SegmentInfos > in LUCENE-2289, so I'll wait for someone to pick it up first, so that I > don't work on it in parallel. > > Thanks, > Shai > > On Sun, Feb 28, 2010 at 1:37 PM, Uwe Schindler <u...@thetaphi.de> wrote: > > I think this is historically. I have seen this in my big 3.0 generification > patches, too. But I did not wanted to change it as Vector has other > allocation schema than ArrayList. But maybe we should simply change it, it’s > a package-private class, right? > > > > But in general subclassing those implementations is not the best thing you > can do. In general the class should extend Object or something else and just > have final field of type List<…>. Exposing the whole API of List to the > outside is bad. > > > > +1 to refactor this class (and don’t let it extend a Collections class). > > > > ----- > > Uwe Schindler > > H.-H.-Meier-Allee 63, D-28213 Bremen > > http://www.thetaphi.de > > eMail: u...@thetaphi.de > > > > *From:* Shai Erera [mailto:ser...@gmail.com] > *Sent:* Sunday, February 28, 2010 12:33 PM > *To:* java-dev@lucene.apache.org > *Subject:* SegmentInfos extends Vector > > > > Hi > > What's the reason SegmentInfos extends Vector rather than say ArrayList? Do > we need the synchronization around it which Vector provides? > > Shai > > >
