lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Doug Cutting <>
Subject Re: incorrect OO in lucene source?
Date Tue, 20 Apr 2004 16:39:05 GMT
Robert Engels wrote:
> Lucene is often cited as an excellent example of OO design.

That is kind, but the primary goal of Lucene is to provide 
functionality, not to use "correct" OO design.  The two are not always 
in accord.

> It seems the abstract base classes were created so there was a place to put
> the static 'helper' methods. Would it not be better to have separate
> interfaces and abstract base classes that the implementations could extend
> if necessary?

Functionally, the only thing an interface has that cannot be implemented 
by a class is multiple inheritance.  Multiple inheritance is useful for 
simple behaviour mixins, like Comparable and Cloneable, but is rarely 
useful with more complex method sets.  So, in cases where every 
implementation needs the same helper methods and multiple inheritance is 
not needed, interfaces add nothing functionally except more code to be 
maintained.  That said, an interface does have some documentation value.

> The main problem seems to be the use of IndexReader in many interfaces, when
> it should be using a Searchable. Otherwise, what is the point of Searchable.

Searchable was added to provide an API appropriate for remote 
invocation, to support RemoteSearchable.

> The following object use chain shows the 'pointlessness' of Searchable:
> Searchable->Query->Weight->IndexReader

IndexReader is required by methods which implement search algorithms, 
run in a potentially remote JVM.  Only the Query and TopDocs are 
transmitted over the wire, so only these are handled by Searchable 
methods.  Other, higher-bandwidth data is handled further downstream in 
the call chain, and thus is processed locally on the remote machine.

> The Query object has the method:
> public Query rewrite(IndexReader reader) throws IOException...
> but the entire search package is based on 'Searchable's, should it not be
> public Query rewrite(Searchable searchable) throws IOException...
> If not, the entire idea of 'external' searchable implementations break,
> because the end up having to 'know' an IndexReader, and IndexReader is a
> baseclass, etc.

IndexReader has a much richer API than Searchable.  It is required for 
rewrite, which can, e.g., iterate over all terms in the index.  Such 
iteration is not appropriate over RPCs.

> The same goes for
> public Weight weight(Searcher searcher);
> should this not be a 'Searchable'?

I think this could be changed without harming anything.  Weights are 
only ever constructed locally, so it wouldn't really make a difference.

> The Weight interface has the following:
> Scorer scorer(IndexReader reader) throws IOException;
> Should it not be
> Scorer scorer(Searchable searchable) throws IOException;
> The same goes for explain().

Again, IndexReader's API is required to implement these methods. 
Searchable has only a very small API, that which can be efficiently 
invoked remotely.

> It seems that Searchable was created because IndexReader was not an
> interface, but the methods have not been changed to use a Searchable instead
> of IndexReader.

No, Searchable and Searcher were created to provide the functionality of 
MultiSearcher and RemoteSearchable.  There are search operations (like 
HitCollector-based searching) which are defined by Searcher, not 
Searchable, that may not be run remotely, but may be invoked locally 
over multiple indexes.

> It seems many of the 'search' package classes use a 'Searcher', when they
> should be using a 'Searchable'. If these core classes really need the
> methods defined in 'Searcher', then these methods should be moved to
> 'Searchable'.

The Searchable interface should be restricted to methods which are 
useful in implementing effective, remotely invokable search.

> Shouldn't 'Filter' just be an interface, with the method
> boolean filter(int docnum);
> as to whether or not the document should be filtered?

How would this make the world a better place?


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

View raw message