Implementing List<?> to throw UOE everywhere is of course a bad idea, but we can do this for backwards compatibility and simple deprecate such use. I just don’t want to break code that calls get(int) and so on. As remove() or clear() was never supported, it should simply throw UOE. If somebody would have done that before, his code was broken either way (this is one problem of exposing the List interface as done by Vector? I just try to resume what the options are. But backwards compatibility would be broken also, if somebody would call a Vector-method like elementAt(int). So implementing List<?> would only be half of the solution.
And of course if it is Iterable, its iterator on the inner implementation should be wrapped on top of Collections.unmodifiableXxx(), as modifying this iterator should not be allowed (as it would open the inner impl’s modification, that is reserved to the class itsself). ----- Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen <http://www.thetaphi.de/> 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 <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 <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
