lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <ser...@gmail.com>
Subject Re: SegmentInfos extends Vector
Date Sun, 28 Feb 2010 12:37:36 GMT
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 <uwe@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: uwe@thetaphi.de
>
>
>
> *From:* Shai Erera [mailto:serera@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 <uwe@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: uwe@thetaphi.de
>
>
>
> *From:* Shai Erera [mailto:serera@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
>
>
>

Mime
View raw message