lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Doug Cutting <>
Subject Re: File timestamps / subclassing of IndexReader
Date Wed, 19 Nov 2003 18:38:56 GMT
Christoph Goller wrote:
> Sorry for the delay. I was busy with other things and it took some
> time to fix the problems that Doug mentioned.

No problem.  Thanks so much for working on this patch.  It looks really 
good, and solves a real problem.  We can't demand that volunteers work 
on a schedule!

My only concerns now are aesthetic.  You add segmentInfos as a protected 
field of IndexReader, which is a public class.  This means it will 
appear in the javadoc.  Yet this field is of type SegmentInfos, a 
package-private class, which will not be in the javadoc.

I try hard to make the javadoc only show things which are meant to be a 
part of the public API.  So the question is, should SegmentInfos be a 
public class, i.e., do we think that folks might need to reference it in 
their application code, or should it remain package private?

My rule is that, if you cannot imagine a reasonable use for something, 
or if there's an already another public way of doing it, don't make it 
public.  Keeping more of the implementation private makes it easier to 
compatibly evolve the implementation.  If they really want, folks can 
change things in their private copy of the sources.  What we want to 
avoid is incompatible changes for folks who only use public APIs.  Over 
the long term, a smaller public API is easier to maintain.

My preference is that SegmentInfos remain package private.  The easiest 
way to do this would just be to make the segmentInfos field of 
IndexReader package private (i.e. with no public/private declaration). 
The only problem is that then IndexReader would not be correctly 
subclassable from outside the package.

Alternately, segmentInfos could be made private, will all access from 
IndexReader only.  Can you see a way to get things to work correctly 
this way?

Finally, you add a new constructor to SegmentsReader but the old 
constructor is no longer used (except by the new constructor).  It would 
be better to simply replace the existing constructor with a new 
constructor.  Less code is better.

If you are tired of working on this, and/or have no more time, please 
say so.  If you are unable, I will try to address the above issues myself.

Thanks again for all your contributions,


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message