Re: svn commit: r164695 - in /lucene/java/trunk: CHANGES.txt src/java/org/apache/lucene/search/Hit.java src/java/org/apache/lucene/search/HitIterator.java src/java/org/apache/lucene/search/Hits.java src/test/org/apache/lucene/TestHitIterator.java

[Otis Gospodnetic-2]

Hello,

Would it be better to explicitly check for out of bounds hitNumber
instead of catching ArrayIndexOutOfBoundsException?

if (hitNumber > hits.length()) {
  throw new NoSuchElementException();
}

Also, is "a future for a hit" a typo, or does that actually mean
something?  This makes me think of Python's "future", but I'm not sure
what this means in this context.

Thanks,
Otis



--- [hidden email] wrote:

> Author: ehatcher
> Date: Mon Apr 25 17:21:53 2005
> New Revision: 164695
>
> URL: http://svn.apache.org/viewcvs?rev=164695&view=rev
> Log:
> Add Hits.iterator and corresponding HitIterator and Hit classes.
> Contributed by Jeremy Rayner
>
> Added:
>     lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java
>    
> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
>     lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
> Modified:
>     lucene/java/trunk/CHANGES.txt
>     lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java
>
> Modified: lucene/java/trunk/CHANGES.txt
> URL:
>

http://svn.apache.org/viewcvs/lucene/java/trunk/CHANGES.txt?rev=164695&r1=164694&r2=164695&view=diff
>
==============================================================================

> --- lucene/java/trunk/CHANGES.txt (original)
> +++ lucene/java/trunk/CHANGES.txt Mon Apr 25 17:21:53 2005
> @@ -91,6 +91,11 @@
>      which lets the caller get the Lucene version information
> specified in
>      the Lucene Jar.
>      (Doug Cutting via Otis)
> +
> +15. Added Hits.iterator() method and corresponding HitIterator and
> Hit objects.
> +    This provides standard java.util.Iterator iteration over Hits.
> +    Each call to the iterator's next() method returns a Hit object.
> +    (Jeremy Rayner via Erik)
>      
>  API Changes
>  
>
> Added: lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java
> URL:
>

http://svn.apache.org/viewcvs/lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java?rev=164695&view=auto
>
==============================================================================

> --- lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java
> (added)
> +++ lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java Mon
> Apr 25 17:21:53 2005
> @@ -0,0 +1,125 @@
> +/**
> + * Copyright 2005 The Apache Software Foundation
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions
> and
> + * limitations under the License.
> + */
> +
> +package org.apache.lucene.search;
> +
> +import java.io.IOException;
> +
> +import org.apache.lucene.document.Document;
> +
> +/**
> + * a lazy future for a hit, useful for iterators over instances of
> Hits
> + *
> + * @author Jeremy Rayner
> + */
> +public class Hit implements java.io.Serializable {
> +
> +  private float score;
> +  private int id;
> +  private Document doc = null;
> +
> +  private boolean resolved = false;
> +
> +  private Hits hits = null;
> +  private int hitNumber;
> +
> +  /**
> +   * Constructed from {@link HitIterator}
> +   * @param hits Hits returned from a search
> +   * @param hitNumber Hit index in Hits
> +   */
> +  Hit(Hits hits, int hitNumber) {
> +    this.hits = hits;
> +    this.hitNumber = hitNumber;
> +  }
> +
> +  /**
> +   * Returns document for this hit.
> +   *
> +   * @see {@link Hits#doc(int)}
> +   */
> +  public Document getDocument() throws IOException {
> +    if (!resolved) fetchTheHit();
> +    return doc;
> +  }
> +
> +  /**
> +   * Returns score for this hit.
> +   *
> +   * @see {@link Hits#score(int)}
> +   */
> +  public float getScore() throws IOException {
> +    if (!resolved) fetchTheHit();
> +    return score;
> +  }
> +
> +  /**
> +   * Returns id for this hit.
> +   *
> +   * @see {@link Hits#id(int)}
> +   */
> +  public int getId() throws IOException {
> +    if (!resolved) fetchTheHit();
> +    return id;
> +  }
> +
> +  private void fetchTheHit() throws IOException {
> +    doc = hits.doc(hitNumber);
> +    score = hits.score(hitNumber);
> +    id = hits.id(hitNumber);
> +    resolved = true;
> +  }
> +
> +  // provide some of the Document style interface (the simple stuff)
> +
> +  /**
> +   * Returns the boost factor for this hit on any field of the
> underlying document.
> +   *
> +   * @see {@link Document#getBoost()}
> +   */
> +  public float getBoost() throws IOException {
> +    return getDocument().getBoost();
> +  }
> +
> +  /**
> +   * Returns the string value of the field with the given name if
> any exist in
> +   * this document, or null.  If multiple fields exist with this
> name, this
> +   * method returns the first value added. If only binary fields
> with this name
> +   * exist, returns null.
> +   *
> +   * @see {@link Document#get(String)}
> +   */
> +  public String get(String name) throws IOException {
> +    return getDocument().get(name);
> +  }
> +
> +  /**
> +   * Prints the fields of the underlying document for human
> consumption.
> +   * <p/>
> +   * If an IOException occurs whilst getting the document, returns
> null
> +   *
> +   * @see {@link Document#toString()}
> +   */
> +  public String toString() {
> +    try {
> +      return getDocument().toString();
> +    } catch (IOException e) {
> +      return null;
> +    }
> +  }
> +
> +
> +}
>
> Added:
> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
> URL:
>

http://svn.apache.org/viewcvs/lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java?rev=164695&view=auto
>
==============================================================================

> ---
> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
> (added)
> +++
> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
> Mon Apr 25 17:21:53 2005
> @@ -0,0 +1,78 @@
> +/**
> + * Copyright 2005 The Apache Software Foundation
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions
> and
> + * limitations under the License.
> + */
> +
> +package org.apache.lucene.search;
> +
> +import java.util.Iterator;
> +import java.util.NoSuchElementException;
> +
> +/**
> + * An iterator over {@link Hits} that provides lazy fetching of each
> document.
> + * {@link Hits#iterator()} returns an instance of this class.  Calls
> to {@link #next()}
> + * return a {@link Hit} instance.
> + *
> + * @author Jeremy Rayner
> + */
> +public class HitIterator implements Iterator {
> +  private Hits hits;
> +  private int hitNumber = 0;
> +
> +  /**
> +   * Constructed from {@link Hits#iterator()}.
> +   */
> +  HitIterator(Hits hits) {
> +    this.hits = hits;
> +  }
> +
> +  /**
> +   * @return true if current hit is less than the total number of
> {@link Hits}.
> +   */
> +  public boolean hasNext() {
> +    return hitNumber < hits.length();
> +  }
> +
> +  /**
> +   * Returns a {@link Hit} instance representing the next hit in
> {@link Hits}.
> +   *
> +   * @return Next {@link Hit}.
> +   */
> +  public Object next() {
> +    try {
> +      Object next = new Hit(hits, hitNumber);
> +      hitNumber++;
> +      return next;
> +    } catch (IndexOutOfBoundsException e) {
> +      throw new NoSuchElementException();
> +    }
> +  }
> +
> +  /**
> +   * Unsupported operation.
> +   *
> +   * @throws UnsupportedOperationException
> +   */
> +  public void remove() {
> +    throw new UnsupportedOperationException();
> +  }
> +
> +  /**
> +   * Returns the total number of hits.
> +   */
> +  public int length() {
> +    return hits.length();
> +  }
> +}
> +
>
> Modified:
> lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java
> URL:
>

http://svn.apache.org/viewcvs/lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java?rev=164695&r1=164694&r2=164695&view=diff
>
==============================================================================

> --- lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java
> (original)
> +++ lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java Mon
> Apr 25 17:21:53 2005
> @@ -18,6 +18,7 @@
>  
>  import java.io.IOException;
>  import java.util.Vector;
> +import java.util.Iterator;
>  
>  import org.apache.lucene.document.Document;
>  
> @@ -114,6 +115,13 @@
>      return hitDoc(n).id;
>    }
>  
> +  /**
> +   * Returns an {@link Iterator} to navigate the Hits.  Each item
> returned
> +   * from {@link Iterator#next()} is a {@link Hit}.
> +   */
> +  public Iterator iterator() {
> +    return new HitIterator(this);
> +  }
>  
>    private final HitDoc hitDoc(int n) throws IOException {
>      if (n >= length) {
>
> Added:
> lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
> URL:
>

http://svn.apache.org/viewcvs/lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java?rev=164695&view=auto
>
==============================================================================

> --- lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
> (added)
> +++ lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
> Mon Apr 25 17:21:53 2005
> @@ -0,0 +1,50 @@
> +package org.apache.lucene;
> +
> +import junit.framework.TestCase;
> +import org.apache.lucene.store.RAMDirectory;
> +import org.apache.lucene.index.IndexWriter;
> +import org.apache.lucene.index.Term;
> +import org.apache.lucene.analysis.WhitespaceAnalyzer;
> +import org.apache.lucene.document.Document;
> +import org.apache.lucene.document.Field;
> +import org.apache.lucene.search.IndexSearcher;
> +import org.apache.lucene.search.TermQuery;
> +import org.apache.lucene.search.Hits;
> +import org.apache.lucene.search.Hit;
> +import org.apache.lucene.search.HitIterator;
> +
> +/**
> + * This test intentionally not put in the search package in order
> + * to test HitIterator and Hit package protection.
> + */
> +public class TestHitIterator extends TestCase {
> +  public void testIterator() throws Exception {
> +    RAMDirectory directory = new RAMDirectory();
> +
> +    IndexWriter writer = new IndexWriter(directory, new
> WhitespaceAnalyzer(), true);
> +    Document doc = new Document();
> +    doc.add(new Field("field", "iterator test doc 1",
> Field.Store.YES, Field.Index.TOKENIZED));
> +    writer.addDocument(doc);
> +
> +    doc = new Document();
> +    doc.add(new Field("field", "iterator test doc 2",
> Field.Store.YES, Field.Index.TOKENIZED));
> +    writer.addDocument(doc);
> +
> +    writer.close();
> +
> +    IndexSearcher searcher = new IndexSearcher(directory);
> +    Hits hits = searcher.search(new TermQuery(new Term("field",
> "iterator")));
> +
> +    HitIterator iterator = (HitIterator) hits.iterator();
> +    assertEquals(2, iterator.length());
> +    assertTrue(iterator.hasNext());
> +    Hit hit = (Hit) iterator.next();
> +    assertEquals("iterator test doc 1", hit.get("field"));
> +
> +    assertTrue(iterator.hasNext());
> +    hit = (Hit) iterator.next();
> +    assertEquals("iterator test doc 2",
> hit.getDocument().get("field"));
> +
> +    assertFalse(iterator.hasNext());
> +  }
> +}
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Erik Hatcher]

On Apr 25, 2005, at 10:36 PM, Otis Gospodnetic wrote:
> Would it be better to explicitly check for out of bounds hitNumber
> instead of catching ArrayIndexOutOfBoundsException?
>
> if (hitNumber > hits.length()) {
>   throw new NoSuchElementException();
> }

Good eye.  I've added a test case that identified this issue since the  
out of bound exception could never have occurred and then fixed the  
issue.

> Also, is "a future for a hit" a typo, or does that actually mean
> something?  This makes me think of Python's "future", but I'm not sure
> what this means in this context.

I've cleared this up.

Thanks,
        Erik

>
> Thanks,
> Otis
>
>
>
> --- [hidden email] wrote:
>
>> Author: ehatcher
>> Date: Mon Apr 25 17:21:53 2005
>> New Revision: 164695
>>
>> URL: http://svn.apache.org/viewcvs?rev=164695&view=rev
>> Log:
>> Add Hits.iterator and corresponding HitIterator and Hit classes.
>> Contributed by Jeremy Rayner
>>
>> Added:
>>     lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java
>>
>> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
>>     lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
>> Modified:
>>     lucene/java/trunk/CHANGES.txt
>>     lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java
>>
>> Modified: lucene/java/trunk/CHANGES.txt
>> URL:
>>
> http://svn.apache.org/viewcvs/lucene/java/trunk/CHANGES.txt?
> rev=164695&r1=164694&r2=164695&view=diff
>>
> =======================================================================
> =======
>> --- lucene/java/trunk/CHANGES.txt (original)
>> +++ lucene/java/trunk/CHANGES.txt Mon Apr 25 17:21:53 2005
>> @@ -91,6 +91,11 @@
>>      which lets the caller get the Lucene version information
>> specified in
>>      the Lucene Jar.
>>      (Doug Cutting via Otis)
>> +
>> +15. Added Hits.iterator() method and corresponding HitIterator and
>> Hit objects.
>> +    This provides standard java.util.Iterator iteration over Hits.
>> +    Each call to the iterator's next() method returns a Hit object.
>> +    (Jeremy Rayner via Erik)
>>
>>  API Changes
>>
>>
>> Added: lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java
>> URL:
>>
> http://svn.apache.org/viewcvs/lucene/java/trunk/src/java/org/apache/ 
> lucene/search/Hit.java?rev=164695&view=auto
>>
> =======================================================================
> =======
>> --- lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java
>> (added)
>> +++ lucene/java/trunk/src/java/org/apache/lucene/search/Hit.java Mon
>> Apr 25 17:21:53 2005
>> @@ -0,0 +1,125 @@
>> +/**
>> + * Copyright 2005 The Apache Software Foundation
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + *     http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing,
>> software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> implied.
>> + * See the License for the specific language governing permissions
>> and
>> + * limitations under the License.
>> + */
>> +
>> +package org.apache.lucene.search;
>> +
>> +import java.io.IOException;
>> +
>> +import org.apache.lucene.document.Document;
>> +
>> +/**
>> + * a lazy future for a hit, useful for iterators over instances of
>> Hits
>> + *
>> + * @author Jeremy Rayner
>> + */
>> +public class Hit implements java.io.Serializable {
>> +
>> +  private float score;
>> +  private int id;
>> +  private Document doc = null;
>> +
>> +  private boolean resolved = false;
>> +
>> +  private Hits hits = null;
>> +  private int hitNumber;
>> +
>> +  /**
>> +   * Constructed from {@link HitIterator}
>> +   * @param hits Hits returned from a search
>> +   * @param hitNumber Hit index in Hits
>> +   */
>> +  Hit(Hits hits, int hitNumber) {
>> +    this.hits = hits;
>> +    this.hitNumber = hitNumber;
>> +  }
>> +
>> +  /**
>> +   * Returns document for this hit.
>> +   *
>> +   * @see {@link Hits#doc(int)}
>> +   */
>> +  public Document getDocument() throws IOException {
>> +    if (!resolved) fetchTheHit();
>> +    return doc;
>> +  }
>> +
>> +  /**
>> +   * Returns score for this hit.
>> +   *
>> +   * @see {@link Hits#score(int)}
>> +   */
>> +  public float getScore() throws IOException {
>> +    if (!resolved) fetchTheHit();
>> +    return score;
>> +  }
>> +
>> +  /**
>> +   * Returns id for this hit.
>> +   *
>> +   * @see {@link Hits#id(int)}
>> +   */
>> +  public int getId() throws IOException {
>> +    if (!resolved) fetchTheHit();
>> +    return id;
>> +  }
>> +
>> +  private void fetchTheHit() throws IOException {
>> +    doc = hits.doc(hitNumber);
>> +    score = hits.score(hitNumber);
>> +    id = hits.id(hitNumber);
>> +    resolved = true;
>> +  }
>> +
>> +  // provide some of the Document style interface (the simple stuff)
>> +
>> +  /**
>> +   * Returns the boost factor for this hit on any field of the
>> underlying document.
>> +   *
>> +   * @see {@link Document#getBoost()}
>> +   */
>> +  public float getBoost() throws IOException {
>> +    return getDocument().getBoost();
>> +  }
>> +
>> +  /**
>> +   * Returns the string value of the field with the given name if
>> any exist in
>> +   * this document, or null.  If multiple fields exist with this
>> name, this
>> +   * method returns the first value added. If only binary fields
>> with this name
>> +   * exist, returns null.
>> +   *
>> +   * @see {@link Document#get(String)}
>> +   */
>> +  public String get(String name) throws IOException {
>> +    return getDocument().get(name);
>> +  }
>> +
>> +  /**
>> +   * Prints the fields of the underlying document for human
>> consumption.
>> +   * <p/>
>> +   * If an IOException occurs whilst getting the document, returns
>> null
>> +   *
>> +   * @see {@link Document#toString()}
>> +   */
>> +  public String toString() {
>> +    try {
>> +      return getDocument().toString();
>> +    } catch (IOException e) {
>> +      return null;
>> +    }
>> +  }
>> +
>> +
>> +}
>>
>> Added:
>> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
>> URL:
>>
> http://svn.apache.org/viewcvs/lucene/java/trunk/src/java/org/apache/ 
> lucene/search/HitIterator.java?rev=164695&view=auto
>>
> =======================================================================
> =======
>> ---
>> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
>> (added)
>> +++
>> lucene/java/trunk/src/java/org/apache/lucene/search/HitIterator.java
>> Mon Apr 25 17:21:53 2005
>> @@ -0,0 +1,78 @@
>> +/**
>> + * Copyright 2005 The Apache Software Foundation
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + *     http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing,
>> software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> implied.
>> + * See the License for the specific language governing permissions
>> and
>> + * limitations under the License.
>> + */
>> +
>> +package org.apache.lucene.search;
>> +
>> +import java.util.Iterator;
>> +import java.util.NoSuchElementException;
>> +
>> +/**
>> + * An iterator over {@link Hits} that provides lazy fetching of each
>> document.
>> + * {@link Hits#iterator()} returns an instance of this class.  Calls
>> to {@link #next()}
>> + * return a {@link Hit} instance.
>> + *
>> + * @author Jeremy Rayner
>> + */
>> +public class HitIterator implements Iterator {
>> +  private Hits hits;
>> +  private int hitNumber = 0;
>> +
>> +  /**
>> +   * Constructed from {@link Hits#iterator()}.
>> +   */
>> +  HitIterator(Hits hits) {
>> +    this.hits = hits;
>> +  }
>> +
>> +  /**
>> +   * @return true if current hit is less than the total number of
>> {@link Hits}.
>> +   */
>> +  public boolean hasNext() {
>> +    return hitNumber < hits.length();
>> +  }
>> +
>> +  /**
>> +   * Returns a {@link Hit} instance representing the next hit in
>> {@link Hits}.
>> +   *
>> +   * @return Next {@link Hit}.
>> +   */
>> +  public Object next() {
>> +    try {
>> +      Object next = new Hit(hits, hitNumber);
>> +      hitNumber++;
>> +      return next;
>> +    } catch (IndexOutOfBoundsException e) {
>> +      throw new NoSuchElementException();
>> +    }
>> +  }
>> +
>> +  /**
>> +   * Unsupported operation.
>> +   *
>> +   * @throws UnsupportedOperationException
>> +   */
>> +  public void remove() {
>> +    throw new UnsupportedOperationException();
>> +  }
>> +
>> +  /**
>> +   * Returns the total number of hits.
>> +   */
>> +  public int length() {
>> +    return hits.length();
>> +  }
>> +}
>> +
>>
>> Modified:
>> lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java
>> URL:
>>
> http://svn.apache.org/viewcvs/lucene/java/trunk/src/java/org/apache/ 
> lucene/search/Hits.java?rev=164695&r1=164694&r2=164695&view=diff
>>
> =======================================================================
> =======
>> --- lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java
>> (original)
>> +++ lucene/java/trunk/src/java/org/apache/lucene/search/Hits.java Mon
>> Apr 25 17:21:53 2005
>> @@ -18,6 +18,7 @@
>>
>>  import java.io.IOException;
>>  import java.util.Vector;
>> +import java.util.Iterator;
>>
>>  import org.apache.lucene.document.Document;
>>
>> @@ -114,6 +115,13 @@
>>      return hitDoc(n).id;
>>    }
>>
>> +  /**
>> +   * Returns an {@link Iterator} to navigate the Hits.  Each item
>> returned
>> +   * from {@link Iterator#next()} is a {@link Hit}.
>> +   */
>> +  public Iterator iterator() {
>> +    return new HitIterator(this);
>> +  }
>>
>>    private final HitDoc hitDoc(int n) throws IOException {
>>      if (n >= length) {
>>
>> Added:
>> lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
>> URL:
>>
> http://svn.apache.org/viewcvs/lucene/java/trunk/src/test/org/apache/ 
> lucene/TestHitIterator.java?rev=164695&view=auto
>>
> =======================================================================
> =======
>> --- lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
>> (added)
>> +++ lucene/java/trunk/src/test/org/apache/lucene/TestHitIterator.java
>> Mon Apr 25 17:21:53 2005
>> @@ -0,0 +1,50 @@
>> +package org.apache.lucene;
>> +
>> +import junit.framework.TestCase;
>> +import org.apache.lucene.store.RAMDirectory;
>> +import org.apache.lucene.index.IndexWriter;
>> +import org.apache.lucene.index.Term;
>> +import org.apache.lucene.analysis.WhitespaceAnalyzer;
>> +import org.apache.lucene.document.Document;
>> +import org.apache.lucene.document.Field;
>> +import org.apache.lucene.search.IndexSearcher;
>> +import org.apache.lucene.search.TermQuery;
>> +import org.apache.lucene.search.Hits;
>> +import org.apache.lucene.search.Hit;
>> +import org.apache.lucene.search.HitIterator;
>> +
>> +/**
>> + * This test intentionally not put in the search package in order
>> + * to test HitIterator and Hit package protection.
>> + */
>> +public class TestHitIterator extends TestCase {
>> +  public void testIterator() throws Exception {
>> +    RAMDirectory directory = new RAMDirectory();
>> +
>> +    IndexWriter writer = new IndexWriter(directory, new
>> WhitespaceAnalyzer(), true);
>> +    Document doc = new Document();
>> +    doc.add(new Field("field", "iterator test doc 1",
>> Field.Store.YES, Field.Index.TOKENIZED));
>> +    writer.addDocument(doc);
>> +
>> +    doc = new Document();
>> +    doc.add(new Field("field", "iterator test doc 2",
>> Field.Store.YES, Field.Index.TOKENIZED));
>> +    writer.addDocument(doc);
>> +
>> +    writer.close();
>> +
>> +    IndexSearcher searcher = new IndexSearcher(directory);
>> +    Hits hits = searcher.search(new TermQuery(new Term("field",
>> "iterator")));
>> +
>> +    HitIterator iterator = (HitIterator) hits.iterator();
>> +    assertEquals(2, iterator.length());
>> +    assertTrue(iterator.hasNext());
>> +    Hit hit = (Hit) iterator.next();
>> +    assertEquals("iterator test doc 1", hit.get("field"));
>> +
>> +    assertTrue(iterator.hasNext());
>> +    hit = (Hit) iterator.next();
>> +    assertEquals("iterator test doc 2",
>> hit.getDocument().get("field"));
>> +
>> +    assertFalse(iterator.hasNext());
>> +  }
>> +}
>>
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jeremy Rayner]

On 4/26/05, Otis Gospodnetic <[hidden email]> wrote:
> Also, is "a future for a hit" a typo, or does that actually mean
> something?  This makes me think of Python's "future", but I'm not sure
> what this means in this context.

My feeling originally was that as the obtaining of the document
was expensive, a Hit should be a bit like the 'Future Value' pattern,
where a Hit is just a promise to delve into Hits with a certain index
at some point in the future.
( see http://c2.com/cgi/wiki?FutureValue )  
Which interestingly enough now seems to be implemented in Doug Lea's
changes for Java 5
( http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/Future.html )

Although without the asynchronous element, I guess it is just lazy
initialization.

An alternative implementation of Hit could be a 'Virtual
Proxy(GOF:207)' that stores
a delegate FutureHit or ActualHit, the FutureHit could be the starting
position, but after any call the delegates reference is swapped over
to ActualHit.  This would eliminate the check of 'resolved' at the start
of each method, and therefore increase perfomance.  However a memory
overhead would be incurred for the overhead of having three classes
instead of one.
So it's a better perfomance vs less memory usage tradeoff.

Thanks for allowing this change, it has now turned my previous Groovy example
( http://javanicus.com/blog2/items/178-index.html ) from

for ( i in 0 ..< hits.length() ) {
    println(hits.doc(i)["filename"])
}

into

hits.each{
    println(it.filename)
}

which has far less chances for making typos :-)

jez.
--
http://javanicus.com/blog2

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Daniel Naber]

On Tuesday 26 April 2005 02:21, [hidden email] wrote:

> +  public String toString() {
> +    try {
> +      return getDocument().toString();
> +    } catch (IOException e) {
> +      return null;
> +    }
> +  }

Wouldn't it be better here to re-throw the exception as a RuntimeException?

Regards
 Daniel

--
http://www.danielnaber.de

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Erik Hatcher]

On Apr 26, 2005, at 2:38 PM, Daniel Naber wrote:

> On Tuesday 26 April 2005 02:21, [hidden email] wrote:
>
>> +  public String toString() {
>> +    try {
>> +      return getDocument().toString();
>> +    } catch (IOException e) {
>> +      return null;
>> +    }
>> +  }
>
> Wouldn't it be better here to re-throw the exception as a
> RuntimeException?

I don't know.... would it?  I have no preference, though it seems ok to
me to simply return null since this is the toString method.  For a
Document, the toString is only useful for debugging anyway.

        Erik


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Daniel Naber-2]

On Tuesday 26 April 2005 21:09, Erik Hatcher wrote:

> I don't know.... would it?  I have no preference, though it seems ok to
> me to simply return null since this is the toString method.  For a
> Document, the toString is only useful for debugging anyway.

Yes, and during debugging it would be especially confusing to just hide the
exception. Sure, people will see that there's a problem with a "null"
document, but then why not show the exception directly?

Regards
 Daniel

--
http://www.danielnaber.de

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[DM Smith]

Erik Hatcher wrote:

> On Apr 26, 2005, at 2:38 PM, Daniel Naber wrote:
>
>> On Tuesday 26 April 2005 02:21, [hidden email] wrote:
>>
>>> +  public String toString() {
>>> +    try {
>>> +      return getDocument().toString();
>>> +    } catch (IOException e) {
>>> +      return null;
>>> +    }
>>> +  }
>>
>>
>> Wouldn't it be better here to re-throw the exception as a
>> RuntimeException?
>
>
> I don't know.... would it?  I have no preference, though it seems ok
> to me to simply return null since this is the toString method.  For a
> Document, the toString is only useful for debugging anyway.

Two thoughts:
If getDocument().toString() cannot possibly throw an IOException, but it
is part of the signature, then it does not matter.

Once lucene is at 1.4, it would be better to use an assert in the catch
and not throw an error but return "" instead of null. The asserts can be
removed at runtime by passing flags to the program. Assertions are best
used for situations that should never happen.

public String toString()
{
    try {
       return getDocument().toString();
    } catch (IOException e) {
       assert false : e;
       return "";
    }
}

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jeremy Rayner]

On 4/26/05, Daniel Naber <[hidden email]> wrote:

> On Tuesday 26 April 2005 21:09, Erik Hatcher wrote:
>
> > I don't know.... would it? I have no preference, though it seems ok to
> > me to simply return null since this is the toString method. For a
> > Document, the toString is only useful for debugging anyway.
>
> Yes, and during debugging it would be especially confusing to just hide the
> exception. Sure, people will see that there's a problem with a "null"
> document, but then why not show the exception directly?
>

Rather than return null, or throw an undesirable RuntimeException from the
toString() method, it may be more useful for the toString() to indicate the
critical parameters of the promised Hit, rather than the String representation
of one of the underlying members.

How about replacing the toString() method in Hit.java with...

  /**
   * Prints the parameters to be used to discover the promised result.
   */
  public String toString() {
      StringBuffer buffer = new StringBuffer();
      buffer.append("Hit<");
      buffer.append(hits.toString());
      buffer.append(" [");
      buffer.append(hitNumber);
      buffer.append("] ");
      if (resolved) {
          buffer.append("resolved");
      } else {
          buffer.append("unresolved");
      }
      buffer.append(">");
      return buffer.toString();
  }


which will return something like
  "Hit<org.apache.lucene.search.Hits@1258b2 [5] unresolved>"

and no RuntimeException or null in sight.

If the user of the API wants to deal with the potential IOException, then
they would write hit.getDocument().toString() and act accordingly.

Hope this helps,

jez.
--
http://javanicus.com/blog2

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Otis Gospodnetic-2]

Thanks for the Future pointers.  I actually used it, but didn't know it
by name.

Otis

--- Jeremy Rayner <[hidden email]> wrote:

> On 4/26/05, Otis Gospodnetic <[hidden email]> wrote:
> > Also, is "a future for a hit" a typo, or does that actually mean
> > something?  This makes me think of Python's "future", but I'm not
> sure
> > what this means in this context.
>
> My feeling originally was that as the obtaining of the document
> was expensive, a Hit should be a bit like the 'Future Value' pattern,
> where a Hit is just a promise to delve into Hits with a certain index
> at some point in the future.
> ( see http://c2.com/cgi/wiki?FutureValue )  
> Which interestingly enough now seems to be implemented in Doug Lea's
> changes for Java 5
> (
>

http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/Future.html

> )
>
> Although without the asynchronous element, I guess it is just lazy
> initialization.
>
> An alternative implementation of Hit could be a 'Virtual
> Proxy(GOF:207)' that stores
> a delegate FutureHit or ActualHit, the FutureHit could be the
> starting
> position, but after any call the delegates reference is swapped over
> to ActualHit.  This would eliminate the check of 'resolved' at the
> start
> of each method, and therefore increase perfomance.  However a memory
> overhead would be incurred for the overhead of having three classes
> instead of one.
> So it's a better perfomance vs less memory usage tradeoff.
>
> Thanks for allowing this change, it has now turned my previous Groovy
> example
> ( http://javanicus.com/blog2/items/178-index.html ) from
>
> for ( i in 0 ..< hits.length() ) {
>     println(hits.doc(i)["filename"])
> }
>
> into
>
> hits.each{
>     println(it.filename)
> }
>
> which has far less chances for making typos :-)
>
> jez.
> --
> http://javanicus.com/blog2
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]