From java-dev-return-47229-apmail-lucene-java-dev-archive=lucene.apache.org@lucene.apache.org Mon Mar 01 07:33:40 2010 Return-Path: Delivered-To: apmail-lucene-java-dev-archive@www.apache.org Received: (qmail 56582 invoked from network); 1 Mar 2010 07:33:39 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 1 Mar 2010 07:33:39 -0000 Received: (qmail 23219 invoked by uid 500); 28 Feb 2010 12:47:00 -0000 Delivered-To: apmail-lucene-java-dev-archive@lucene.apache.org Received: (qmail 23185 invoked by uid 500); 28 Feb 2010 12:47:00 -0000 Mailing-List: contact java-dev-help@lucene.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: java-dev@lucene.apache.org Delivered-To: mailing list java-dev@lucene.apache.org Received: (qmail 23177 invoked by uid 99); 28 Feb 2010 12:47:00 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 28 Feb 2010 12:47:00 +0000 X-ASF-Spam-Status: No, hits=3.4 required=10.0 tests=HTML_MESSAGE,SPF_NEUTRAL X-Spam-Check-By: apache.org Received-SPF: neutral (nike.apache.org: local policy) Received: from [85.25.71.29] (HELO mail.troja.net) (85.25.71.29) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 28 Feb 2010 12:46:48 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.troja.net (Postfix) with ESMTP id 31612D3600D for ; Sun, 28 Feb 2010 13:46:28 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at mail.troja.net Received: from mail.troja.net ([127.0.0.1]) by localhost (megaira.troja.net [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dfYsczDkSMQC for ; Sun, 28 Feb 2010 13:46:25 +0100 (CET) Received: from VEGA (port-83-236-62-54.dynamic.qsc.de [83.236.62.54]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mail.troja.net (Postfix) with ESMTPSA id A1170D36001 for ; Sun, 28 Feb 2010 13:46:24 +0100 (CET) From: "Uwe Schindler" To: References: <786fde51002280333w484971abx6ebd63ca5cee4bbc@mail.gmail.com> <003c01cab86a$6063fc50$212bf4f0$@de> <786fde51002280420j4772fb04x2ce7abcaaa658043@mail.gmail.com> <000301cab871$199c2840$4cd478c0$@de> <786fde51002280437o774c6cabmed16d0333ca55832@mail.gmail.com> In-Reply-To: <786fde51002280437o774c6cabmed16d0333ca55832@mail.gmail.com> Subject: RE: SegmentInfos extends Vector Date: Sun, 28 Feb 2010 13:46:38 +0100 Message-ID: <000e01cab874$11bbadf0$353309d0$@de> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_NextPart_000_000F_01CAB87C.738015F0" X-Mailer: Microsoft Office Outlook 12.0 Thread-Index: Acq4cuTs3U/xycsyTomqiHdl3SztkwAAHKhQ Content-Language: de X-Virus-Checked: Checked by ClamAV on apache.org ------=_NextPart_000_000F_01CAB87C.738015F0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=99t 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. =20 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=E2=80=99s modification, that is = reserved to the class itsself). =20 ----- Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de eMail: uwe@thetaphi.de =20 From: Shai Erera [mailto:serera@gmail.com]=20 Sent: Sunday, February 28, 2010 1:38 PM To: java-dev@lucene.apache.org Subject: Re: SegmentInfos extends Vector =20 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 wrote: I think you should open an issue! I like this refactoring, maybe we can = still let it implement List but only deprecated and most = methods should throw UOE. Just keep get() and so on. =20 ----- Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de =20 eMail: uwe@thetaphi.de =20 From: Shai Erera [mailto:serera@gmail.com]=20 Sent: Sunday, February 28, 2010 1:20 PM To: java-dev@lucene.apache.org Subject: Re: SegmentInfos extends Vector =20 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 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=E2=80=99s a package-private class, right? =20 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<=E2=80=A6>. Exposing the whole = API of List to the outside is bad. =20 +1 to refactor this class (and don=E2=80=99t let it extend a Collections = class). =20 ----- Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de =20 eMail: uwe@thetaphi.de =20 From: Shai Erera [mailto:serera@gmail.com]=20 Sent: Sunday, February 28, 2010 12:33 PM To: java-dev@lucene.apache.org Subject: SegmentInfos extends Vector =20 Hi What's the reason SegmentInfos extends Vector rather than say ArrayList? = Do we need the synchronization around it which Vector provides? Shai =20 =20 ------=_NextPart_000_000F_01CAB87C.738015F0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable

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=E2=80=99t 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=E2=80=99s modification, that is reserved to the class = itsself).

 

-----

Uwe Schindler

H.-H.-Meier-Allee 63, D-28213 = Bremen

http://www.thetaphi.de<= /p>

eMail: uwe@thetaphi.de

 

From:= Shai Erera [mailto:serera@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 = <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

Subject: Re: SegmentInfos extends = Vector

 <= /o:p>

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=E2=80=99s 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<=E2=80=A6>. Exposing the whole API of List to the outside is = bad.

 

+1 to refactor = this class (and don=E2=80=99t 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

 <= /o:p>

Hi

What's the reason SegmentInfos extends Vector rather than say ArrayList? = Do we need the synchronization around it which Vector provides?

Shai

 <= /o:p>

 

------=_NextPart_000_000F_01CAB87C.738015F0--