lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Engels" <reng...@ix.netcom.com>
Subject RE: incorrect OO in lucene source?
Date Tue, 20 Apr 2004 12:48:57 GMT
Not exactly. Nothing is "preventing" me, it just seemed a bit "wrong". I am
involved in a quite large development project which uses Lucene in a small
capacity. I would have liked to leave behind code that was easier to
maintain/understand - I think the proper use of interfaces would have
helped.

We have an 'implementation' of IndexReader that uses a RDBMS. I created the
implementation based on the 1.4rc1 codebase. With 1.4.rc2 the TermVector
stuff was added and I've been reluctant to update the code, for fear of
having to implement many more methods/algs.

Without clear interface definitions it is difficult to know at a glance
"what is expected".

It would have seemed that I would have originally just needed to implement
something like IndexReader, IndexWriter, (with the help of some abstract
base classes) and I would be ok, but neither are interfaces, so it was not
that easy. But on second glance, it looked like am implementation of
Searchable (to get access to the 'search engine') would have been best, but
then I saw the use chain that required IndexReader anyway.

It's not that big of deal. Our code works, and as long as we don't try to
"keep up" with Lucene, we should be ok.

Thanks,
Robert

-----Original Message-----
From: Erik Hatcher [mailto:erik@ehatchersolutions.com]
Sent: Tuesday, April 20, 2004 4:36 AM
To: Lucene Developers List
Subject: Re: incorrect OO in lucene source?


Robert,

I agree with your sentiment about interfaces.  I haven't looked into
the specifics of what you mention about IndexReader though.

Out of curiosity - is there something about Lucene's design that is
preventing you from extending Lucene?  Pragmatically speaking, we
shouldn't change anything unless there is a compelling reason to do so,
and while a clean architecture is a nice ideal, if it ain't broke don't
fix it :)

	Erik


On Apr 19, 2004, at 8:12 PM, Robert Engels wrote:

> Lucene is often cited as an excellent example of OO design.
>
> I see several areas where this does not seem to be the case, mainly
> because
> of the Lucene code uses abstract base classes where it should be using
> interfaces, and other cases, where an interface makes reference to an
> abstract base class as a method/return parameter.
>
> 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?
>
> Some examples are listed below. Am I missing something, or is the code
> structured this way for a reason?
>
> -----
>
> 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.
>
> The following object use chain shows the 'pointlessness' of Searchable:
>
> Searchable->Query->Weight->IndexReader
>
> -----
>
> 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.
>
> The same goes for
>
> public Weight weight(Searcher searcher);
>
> should this not be a 'Searchable'?
>
> -----
>
> 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().
>
> -----
>
> 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.
>
> -----
>
> 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'.
>
> -----
>
> Shouldn't 'Filter' just be an interface, with the method
>
> boolean filter(int docnum);
>
> as to whether or not the document should be filtered?
>
> -----
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: lucene-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: lucene-dev-help@jakarta.apache.org


Mime
View raw message