IIRC: The early implementation of Vector did not extend AbstractList and thus did not have remove.
On Feb 28, 2010, at 8:04 AM, Shai Erera wrote: > Why do you say remove was unsupported before? I don't see it in the class's > impl. It just inherits from Vector and so remove is supported by inheritance. > Since the class is public, someone may have called it. > > Even if we change the class to impl List, period, we'll break back-compat, > just because of the synchronization Vector offers. If anyone out there relies > on that, it's a problem. > > On one hand, the best way would be is to impl Collection, as then someone > will be able to use Collections.synchronizedCollection if one needs it, or > call toArray etc. But Collection does not have a get(index) method, which > might be required and useful ... > > All in all, I don't feel like SegmentInfos is a true collection (even though > its Javadoc starts with "a collection ...". It adds lots of segments related > methods. The collection's ones are really get and iterator? So maybe we > should just impl Iterable and expose whatever API we feel is necessary? > Back-compat wise, if we change anything in this class's extension/implements > details, we break it. > > Unless the folks here don't think we should go to great lengths w/ this > class, and do whatever changes we dim are necessary, even at the cost of > breaking back-compat. And I'd vote that whether with this class or the new > one, we mark it as @lucene.internal. > > Shai > > On Sun, Feb 28, 2010 at 2:49 PM, Uwe Schindler <u...@thetaphi.de> wrote: > Hi Shai, > > > I forgot to mention: Iterable is always a good idea. E.g. during my 3.0 > generification, I made “BooleanQuery implements Iterable<BooleanClause>” and > so on. That makes look the code nice J. Also other classes got this interface > in Lucene. Also adding j.io.Closeable everywhere was a good idea. > > > Uwe > > > ----- > > 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:38 PM > > > To: java-dev@lucene.apache.org > Subject: Re: SegmentInfos extends Vector > > > 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 > > > >
