You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by karl wettin <ka...@gmail.com> on 2007/08/09 15:38:32 UTC

why the mixed scopes?

Is there a reason for writers to be created via the constructor and  
the readers to be created via the static scope?

Would it not be much more beautiful and object oriented to handle  
this using factory methods in Directory? Or perhaps even one layer  
further down? The patch below will allow for alternative stores  
(read: LUCENE-550) without changeing more than a single line of code  
(given that code uses the factory methods).

Also, it would make it much simple to augment Lucene with decorative  
layers such as notification schemes, et c.

According to /me/ the static scope is a violation of pretty much  
everything.


Index: src/java/org/apache/lucene/store/Index.java
===================================================================
--- src/java/org/apache/lucene/store/Index.java (revision 0)
+++ src/java/org/apache/lucene/store/Index.java (revision 0)
@@ -0,0 +1,35 @@
+package org.apache.lucene.store;
+
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.analysis.Analyzer;
+
+import java.io.IOException;
+
+/**
+ * A top level store class with factory methods for reader and writer.
+ */
+public abstract class Index {
+
+  public abstract IndexReader indexReaderFactory() throws IOException;
+  public IndexWriter indexWriterFactory(Analyzer analyzer) throws  
IOException {
+    return indexWriterFactory(analyzer, false);
+  }
+  public abstract IndexWriter indexWriterFactory(Analyzer analyzer,  
boolean create) throws IOException;
+
+}
Index: src/java/org/apache/lucene/store/Directory.java
===================================================================
--- src/java/org/apache/lucene/store/Directory.java     (revision  
564022)
+++ src/java/org/apache/lucene/store/Directory.java     (working copy)
@@ -17,6 +17,10 @@
   * limitations under the License.
   */
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.analysis.Analyzer;
+
import java.io.IOException;
/** A Directory is a flat list of files.  Files may be written once,  
when they
@@ -36,8 +40,16 @@
   *
   * @author Doug Cutting
   */
-public abstract class Directory {
+public abstract class Directory extends Index {
+
+  public IndexReader indexReaderFactory() throws IOException {
+    return IndexReader.open(this);
+  }
+  public IndexWriter indexWriterFactory(Analyzer analyzer, boolean  
create) throws IOException {
+    return new IndexWriter(this, analyzer, create);
+  }
+
    /** Holds the LockFactory instance (implements locking for
     * this Directory instance). */
    protected LockFactory lockFactory;



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


Re: why the mixed scopes?

Posted by karl wettin <ka...@gmail.com>.
I'm not sure whether or not this was a way too lame question and  
suggestion.

Still, I think it makes sense.


9 aug 2007 kl. 15.38 skrev karl wettin:

> Is there a reason for writers to be created via the constructor and  
> the readers to be created via the static scope?
>
> Would it not be much more beautiful and object oriented to handle  
> this using factory methods in Directory? Or perhaps even one layer  
> further down? The patch below will allow for alternative stores  
> (read: LUCENE-550) without changeing more than a single line of  
> code (given that code uses the factory methods).
>
> Also, it would make it much simple to augment Lucene with  
> decorative layers such as notification schemes, et c.
>
> According to /me/ the static scope is a violation of pretty much  
> everything.
>
>
> Index: src/java/org/apache/lucene/store/Index.java
> ===================================================================
> --- src/java/org/apache/lucene/store/Index.java (revision 0)
> +++ src/java/org/apache/lucene/store/Index.java (revision 0)
> @@ -0,0 +1,35 @@
> +package org.apache.lucene.store;
> +
> +import org.apache.lucene.index.IndexReader;
> +import org.apache.lucene.index.IndexWriter;
> +import org.apache.lucene.analysis.Analyzer;
> +
> +import java.io.IOException;
> +
> +/**
> + * A top level store class with factory methods for reader and  
> writer.
> + */
> +public abstract class Index {
> +
> +  public abstract IndexReader indexReaderFactory() throws  
> IOException;
> +  public IndexWriter indexWriterFactory(Analyzer analyzer) throws  
> IOException {
> +    return indexWriterFactory(analyzer, false);
> +  }
> +  public abstract IndexWriter indexWriterFactory(Analyzer  
> analyzer, boolean create) throws IOException;
> +
> +}
> Index: src/java/org/apache/lucene/store/Directory.java
> ===================================================================
> --- src/java/org/apache/lucene/store/Directory.java     (revision  
> 564022)
> +++ src/java/org/apache/lucene/store/Directory.java     (working copy)
> @@ -17,6 +17,10 @@
>   * limitations under the License.
>   */
> +import org.apache.lucene.index.IndexReader;
> +import org.apache.lucene.index.IndexWriter;
> +import org.apache.lucene.analysis.Analyzer;
> +
> import java.io.IOException;
> /** A Directory is a flat list of files.  Files may be written  
> once, when they
> @@ -36,8 +40,16 @@
>   *
>   * @author Doug Cutting
>   */
> -public abstract class Directory {
> +public abstract class Directory extends Index {
> +
> +  public IndexReader indexReaderFactory() throws IOException {
> +    return IndexReader.open(this);
> +  }
> +  public IndexWriter indexWriterFactory(Analyzer analyzer, boolean  
> create) throws IOException {
> +    return new IndexWriter(this, analyzer, create);
> +  }
> +
>    /** Holds the LockFactory instance (implements locking for
>     * this Directory instance). */
>    protected LockFactory lockFactory;
>
>


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


Re: why the mixed scopes?

Posted by karl wettin <ka...@gmail.com>.
(gmail seem to have replaced their dns, sorry if this mail was
repeatedly sent to the list.)


14 aug 2007 kl. 01.08 skrev Chris Hostetter:

> i agree with you that it would be nice if Factory patterns were  
> used more
> (and more consistently) ... but i'm not sure if Directory is the right
> place to put them .. in theor a Directory is a purely abstract data
> storage object that doesn't evne know it contains an index.

Related, I would also like to see an abstract or interface reader and
writer that has nothing to do with Directory.

(Ceterum censeo, Carthago delenda est.)

> I do like the spirit of your patch -- but I don't see any reason  
> not to go
> about solving the same problem using IndexReaderFactory and
> IndexWriterFactory interfaces, with things like IndexReader.open
> delegating to the "default" IndexReaderFactory (specified by a system
> property or something) for backwards compatibility.

The patch is not my limit. It was just something I posted to show in  
code
what it is I'm looking for.

As far as I know this is an ego-thing as I'm the only one that "require"
this feature. It would make it much simpler for me to ensure that my
ad hoc store pass the same tests as the Directory. All the decorative
things I've posted have been nothing but ideas on what else one could
use it for.

With factories in place I would feel more confident refactoring all  
tests
to abstract classes with one implementation per store. Or something  
smarter.
Huge job, but in the end it would make mine and future store  
implementors
life much simpler.


-- 
karl

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


Re: why the mixed scopes?

Posted by Chris Hostetter <ho...@fucit.org>.
: Is there a reason for writers to be created via the constructor and
: the readers to be created via the static scope?

I wasn't arround when those APIs were created, but i believe the main
reason for the static IndexReader.open method was to deal with the unknown
question: will this be a SegmentReader or a MultiReader ?

In essence, it is the factory pattern -- but "poorly" implemented as a
static method (i say "poorly" because it's a subjective opinion)

IndexWriter on the other hand was implemented with a simpler constructor
API since it doesn't have these issues.  (I suspect once upon a time
IndexReader worked the same way, and the public constructor was removed
once the static method was added -- but i haven't verified this in the
anals of commit logs)

: Would it not be much more beautiful and object oriented to handle
: this using factory methods in Directory? Or perhaps even one layer

i agree with you that it would be nice if Factory patterns were used more
(and more consistently) ... but i'm not sure if Directory is the right
place to put them .. in theor a Directory is a purely abstract data
storage object that doesn't evne know it contains an index.  I could
arguable wnt to use the same Direcotry object to construct an IndexWriter,
an IndexReader and to store ancilary data i want to store using the
openInput method.

I do like the spirit of your patch -- but I don't see any reason not to go
about solving the same problem using IndexReaderFactory and
IndexWriterFactory interfaces, with things like IndexReader.open
delegating to the "default" IndexReaderFactory (specified by a system
property or something) for backwards compatibility.

-Hoss


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