You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucenenet.apache.org by ni...@apache.org on 2016/11/25 11:07:23 UTC

[18/26] lucenenet git commit: Repaired several issues that were causing tests to fail. Refactored some of the API, added some missing documentation, and set member accessibility on some members.

Repaired several issues that were causing tests to fail. Refactored some of the API, added some missing documentation, and set member accessibility on some members.


Project: http://git-wip-us.apache.org/repos/asf/lucenenet/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucenenet/commit/df3f64d7
Tree: http://git-wip-us.apache.org/repos/asf/lucenenet/tree/df3f64d7
Diff: http://git-wip-us.apache.org/repos/asf/lucenenet/diff/df3f64d7

Branch: refs/heads/master
Commit: df3f64d718a58dfa43943b9e7ddf25e6e118a4d5
Parents: d8c7353
Author: Shad Storhaug <sh...@shadstorhaug.com>
Authored: Wed Nov 16 20:57:17 2016 +0700
Committer: Shad Storhaug <sh...@shadstorhaug.com>
Committed: Thu Nov 17 22:14:19 2016 +0700

----------------------------------------------------------------------
 src/Lucene.Net.Core/Search/DocIdSetIterator.cs  |   2 +-
 src/Lucene.Net.Core/Util/Bits.cs                |   2 +-
 src/Lucene.Net.Spatial/DisjointSpatialFilter.cs |   8 +-
 .../Prefix/AbstractPrefixTreeFilter.cs          |  24 ++-
 .../Prefix/AbstractVisitingPrefixTreeFilter.cs  | 160 ++++++++--------
 .../Prefix/ContainsPrefixTreeFilter.cs          |  81 ++++----
 .../Prefix/IntersectsPrefixTreeFilter.cs        |   3 -
 .../Prefix/PrefixTreeStrategy.cs                |  11 +-
 .../Prefix/RecursivePrefixTreeStrategy.cs       |   5 +-
 .../Prefix/TermQueryPrefixTreeStrategy.cs       |   7 +-
 src/Lucene.Net.Spatial/Prefix/Tree/Cell.cs      |  54 ++----
 .../Prefix/Tree/GeohashPrefixTree.cs            |  75 ++++----
 .../Prefix/Tree/QuadPrefixTree.cs               | 183 +++++++++----------
 .../Prefix/Tree/SpatialPrefixTree.cs            |  22 +--
 .../Prefix/Tree/SpatialPrefixTreeFactory.cs     |  47 ++---
 .../Prefix/WithinPrefixTreeFilter.cs            |  81 ++++----
 src/Lucene.Net.Spatial/Query/SpatialArgs.cs     |  40 ++--
 .../Query/SpatialArgsParser.cs                  | 101 ++++++----
 .../Query/SpatialOperation.cs                   |  39 ++--
 .../Query/UnsupportedSpatialOperation.cs        |   2 +-
 .../Serialized/SerializedDVStrategy.cs          |  20 +-
 src/Lucene.Net.Spatial/SpatialStrategy.cs       |  31 +++-
 .../Util/CachingDoubleValueSource.cs            |  40 ++--
 .../Util/DistanceToShapeValueSource.cs          |  56 +++---
 src/Lucene.Net.Spatial/Util/ShapeFieldCache.cs  |  10 +-
 .../Util/ShapeFieldCacheDistanceValueSource.cs  |  36 ++--
 .../Util/ShapeFieldCacheProvider.cs             |  21 +--
 .../Util/ShapePredicateValueSource.cs           |  55 +++---
 .../Util/ValueSourceFilter.cs                   |  16 +-
 .../Vector/DistanceValueSource.cs               |  80 ++++----
 .../Vector/PointVectorStrategy.cs               | 119 ++++++------
 .../Prefix/SpatialOpRecursivePrefixTreeTest.cs  |   1 +
 32 files changed, 743 insertions(+), 689 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Core/Search/DocIdSetIterator.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Core/Search/DocIdSetIterator.cs b/src/Lucene.Net.Core/Search/DocIdSetIterator.cs
index 4ec0fec..b89b10f 100644
--- a/src/Lucene.Net.Core/Search/DocIdSetIterator.cs
+++ b/src/Lucene.Net.Core/Search/DocIdSetIterator.cs
@@ -87,7 +87,7 @@ namespace Lucene.Net.Search
         ///
         /// @since 2.9
         /// </summary>
-        public abstract int DocID();
+        public abstract int DocID(); // LUCENENET TODO: Change to property getter
 
         /// <summary>
         /// Advances to the next document in the set and returns the doc it is

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Core/Util/Bits.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Core/Util/Bits.cs b/src/Lucene.Net.Core/Util/Bits.cs
index ac60e78..26866e2 100644
--- a/src/Lucene.Net.Core/Util/Bits.cs
+++ b/src/Lucene.Net.Core/Util/Bits.cs
@@ -34,7 +34,7 @@ namespace Lucene.Net.Util
 
         /// <summary>
         /// Returns the number of bits in this set </summary>
-        int Length();
+        int Length(); // LUCENENET TODO: Change to property getter
     }
 
     public static class BitsHelpers

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/DisjointSpatialFilter.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/DisjointSpatialFilter.cs b/src/Lucene.Net.Spatial/DisjointSpatialFilter.cs
index 41842af..6f2fdd9 100644
--- a/src/Lucene.Net.Spatial/DisjointSpatialFilter.cs
+++ b/src/Lucene.Net.Spatial/DisjointSpatialFilter.cs
@@ -48,15 +48,13 @@ namespace Lucene.Net.Spatial
         /// <param name="args">Used in spatial intersection</param>
         /// <param name="field">
         /// This field is used to determine which docs have spatial data via
-        /// <see cref="Org.Apache.Lucene.Search.FieldCache.GetDocsWithField(Org.Apache.Lucene.Index.AtomicReader, string)
-        /// 	">Org.Apache.Lucene.Search.FieldCache.GetDocsWithField(Org.Apache.Lucene.Index.AtomicReader, string)
-        /// 	</see>
-        /// .
+        /// <see cref="FieldCache.GetDocsWithField(AtomicReader, string)"/>.
         /// Passing null will assume all docs have spatial data.
         /// </param>
         public DisjointSpatialFilter(SpatialStrategy strategy, SpatialArgs args, string field)
         {
             this.field = field;
+
             // TODO consider making SpatialArgs cloneable
             SpatialOperation origOp = args.Operation; //copy so we can restore
             args.Operation = SpatialOperation.Intersects; //temporarily set to intersects
@@ -109,11 +107,13 @@ namespace Lucene.Net.Spatial
                 // intersects filter against the world bounds. So do we add a method to the
                 // strategy, perhaps?  But the strategy can't cache it.
                 docsWithField = FieldCache.DEFAULT.GetDocsWithField((context.AtomicReader), field);
+
                 int maxDoc = context.AtomicReader.MaxDoc;
                 if (docsWithField.Length() != maxDoc)
                 {
                     throw new InvalidOperationException("Bits length should be maxDoc (" + maxDoc + ") but wasn't: " + docsWithField);
                 }
+
                 if (docsWithField is Bits_MatchNoBits)
                 {
                     return null;//match nothing

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/AbstractPrefixTreeFilter.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/AbstractPrefixTreeFilter.cs b/src/Lucene.Net.Spatial/Prefix/AbstractPrefixTreeFilter.cs
index 0591a35..666642a 100644
--- a/src/Lucene.Net.Spatial/Prefix/AbstractPrefixTreeFilter.cs
+++ b/src/Lucene.Net.Spatial/Prefix/AbstractPrefixTreeFilter.cs
@@ -84,7 +84,7 @@ namespace Lucene.Net.Spatial.Prefix
         /// </summary>
         public abstract class BaseTermsEnumTraverser
         {
-            private readonly AbstractPrefixTreeFilter outerInstance;
+            protected readonly AbstractPrefixTreeFilter outerInstance;
             protected readonly AtomicReaderContext context;
             protected Bits acceptDocs;
             protected readonly int maxDoc;
@@ -92,8 +92,6 @@ namespace Lucene.Net.Spatial.Prefix
             protected TermsEnum termsEnum;//remember to check for null in getDocIdSet
             protected DocsEnum docsEnum;
             
-
-            /// <exception cref="System.IO.IOException"></exception>
             public BaseTermsEnumTraverser(AbstractPrefixTreeFilter outerInstance, AtomicReaderContext context, Bits acceptDocs)
             {
                 this.outerInstance = outerInstance;
@@ -102,14 +100,13 @@ namespace Lucene.Net.Spatial.Prefix
                 AtomicReader reader = context.AtomicReader;
                 this.acceptDocs = acceptDocs;
                 maxDoc = reader.MaxDoc;
-                Terms terms = reader.Terms(this.outerInstance.fieldName);
+                Terms terms = reader.Terms(outerInstance.fieldName);
                 if (terms != null)
                 {
                     termsEnum = terms.Iterator(null);
                 }
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             protected virtual void CollectDocs(FixedBitSet bitSet)
             {
                 //WARN: keep this specialization in sync
@@ -124,5 +121,22 @@ namespace Lucene.Net.Spatial.Prefix
         }
 
         #endregion
+
+        /* Eventually uncomment when needed.
+
+        protected void collectDocs(Collector collector) throws IOException {
+          //WARN: keep this specialization in sync
+          assert termsEnum != null;
+          docsEnum = termsEnum.docs(acceptDocs, docsEnum, DocsEnum.FLAG_NONE);
+          int docid;
+          while ((docid = docsEnum.nextDoc()) != DocIdSetIterator.NO_MORE_DOCS) {
+            collector.collect(docid);
+          }
+        }
+
+        public abstract class Collector {
+          abstract void collect(int docid) throws IOException;
+        }
+        */
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/AbstractVisitingPrefixTreeFilter.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/AbstractVisitingPrefixTreeFilter.cs b/src/Lucene.Net.Spatial/Prefix/AbstractVisitingPrefixTreeFilter.cs
index 3118351..d3aa5df 100644
--- a/src/Lucene.Net.Spatial/Prefix/AbstractVisitingPrefixTreeFilter.cs
+++ b/src/Lucene.Net.Spatial/Prefix/AbstractVisitingPrefixTreeFilter.cs
@@ -43,7 +43,7 @@ namespace Lucene.Net.Spatial.Prefix
         // Historical note: this code resulted from a refactoring of RecursivePrefixTreeFilter,
         // which in turn came out of SOLR-2155
 
-        protected internal readonly int prefixGridScanLevel;
+        protected internal readonly int prefixGridScanLevel;//at least one less than grid.getMaxLevels()
 
         public AbstractVisitingPrefixTreeFilter(IShape queryShape, string fieldName, SpatialPrefixTree grid, 
                                                 int detailLevel, int prefixGridScanLevel)
@@ -59,64 +59,18 @@ namespace Lucene.Net.Spatial.Prefix
             {
                 return false;//checks getClass == o.getClass & instanceof
             }
-            
-            var that = (AbstractVisitingPrefixTreeFilter)o;
-            if (prefixGridScanLevel != that.prefixGridScanLevel)
-            {
-                return false;
-            }
+
+            //Ignore prefixGridScanLevel as it is merely a tuning parameter.
+
             return true;
         }
 
         public override int GetHashCode()
         {
             int result = base.GetHashCode();
-            result = 31 * result + prefixGridScanLevel;
             return result;
         }
 
-        #region Nested type: VNode
-
-        /// <summary>
-        /// A Visitor Cell/Cell found via the query shape for
-        /// <see cref="VisitorTemplate">VisitorTemplate</see>
-        /// .
-        /// Sometimes these are reset(cell). It's like a LinkedList node but forms a
-        /// tree.
-        /// </summary>
-        /// <lucene.internal></lucene.internal>
-        public class VNode
-        {
-            internal readonly VNode parent;
-
-            internal Cell cell;
-            internal IEnumerator<VNode> children;
-
-            /// <summary>call reset(cell) after to set the cell.</summary>
-            /// <remarks>call reset(cell) after to set the cell.</remarks>
-            internal VNode(VNode parent)
-            {
-                //Note: The VNode tree adds more code to debug/maintain v.s. a flattened
-                // LinkedList that we used to have. There is more opportunity here for
-                // custom behavior (see preSiblings & postSiblings) but that's not
-                // leveraged yet. Maybe this is slightly more GC friendly.
-                //only null at the root
-                //null, then sometimes set, then null
-                //not null (except initially before reset())
-                // remember to call reset(cell) after
-                this.parent = parent;
-            }
-
-            internal virtual void Reset(Cell cell)
-            {
-                Debug.Assert(cell != null);
-                this.cell = cell;
-                Debug.Assert(children == null);
-            }
-        }
-
-        #endregion
-
         #region Nested type: VisitorTemplate
 
         /// <summary>
@@ -159,7 +113,25 @@ namespace Lucene.Net.Spatial.Prefix
         /// <lucene.internal></lucene.internal>
         public abstract class VisitorTemplate : BaseTermsEnumTraverser
         {
-            private readonly AbstractVisitingPrefixTreeFilter outerInstance;
+            /* Future potential optimizations:
+
+            * Can a polygon query shape be optimized / made-simpler at recursive depths
+              (e.g. intersection of shape + cell box)
+
+            * RE "scan" vs divide & conquer performance decision:
+              We should use termsEnum.docFreq() as an estimate on the number of places at
+              this depth.  It would be nice if termsEnum knew how many terms
+              start with the current term without having to repeatedly next() & test to find out.
+
+            * Perhaps don't do intermediate seek()'s to cells above detailLevel that have Intersects
+              relation because we won't be collecting those docs any way.  However seeking
+              does act as a short-circuit.  So maybe do some percent of the time or when the level
+              is above some threshold.
+
+            * Each shape.relate(otherShape) result could be cached since much of the same relations
+              will be invoked when multiple segments are involved.
+
+            */
 
             protected internal readonly bool hasIndexedLeaves;//if false then we can skip looking for them
 
@@ -173,7 +145,6 @@ namespace Lucene.Net.Spatial.Prefix
                                    bool hasIndexedLeaves)
                 : base(outerInstance, context, acceptDocs)
             {
-                this.outerInstance = outerInstance;
                 this.hasIndexedLeaves = hasIndexedLeaves;
             }
 
@@ -187,9 +158,9 @@ namespace Lucene.Net.Spatial.Prefix
                 //advance
                 if ((thisTerm = termsEnum.Next()) == null)
                 {
-                    return null;
+                    return null;// all done
                 }
-                // all done
+                
                 curVNode = new VNode(null);
                 curVNode.Reset(outerInstance.grid.WorldCell);
 
@@ -225,9 +196,8 @@ namespace Lucene.Net.Spatial.Prefix
                         {
                             if (parentVNode == null)
                             {
-                                goto main_break;
+                                goto main_break;// all done
                             }
-                            // all done
                             if (parentVNode.children.MoveNext())
                             {
                                 //advance next sibling
@@ -261,9 +231,8 @@ namespace Lucene.Net.Spatial.Prefix
                             TermsEnum.SeekStatus seekStatus = termsEnum.SeekCeil(curVNodeTerm);
                             if (seekStatus == TermsEnum.SeekStatus.END)
                             {
-                                break;
+                                break;// all done
                             }
-                            // all done
                             thisTerm = termsEnum.Term();
                             if (seekStatus == TermsEnum.SeekStatus.NOT_FOUND)
                             {
@@ -275,19 +244,17 @@ namespace Lucene.Net.Spatial.Prefix
                         //advance
                         if ((thisTerm = termsEnum.Next()) == null)
                         {
-                            break;
+                            break;// all done
                         }
-                        // all done
                         if (descend)
                         {
                             AddIntersectingChildren();
                         }
                     }
                     ;
-                }
-            main_break:
-                ;
-                //main loop
+                }//main loop
+                main_break: { }
+                
                 return Finish();
             }
 
@@ -319,32 +286,33 @@ namespace Lucene.Net.Spatial.Prefix
                         //advance
                         if ((thisTerm = termsEnum.Next()) == null)
                         {
-                            return;
+                            return;// all done
                         }
                     }
                 }
-                // all done
+                
                 //Decide whether to continue to divide & conquer, or whether it's time to
                 // scan through terms beneath this cell.
                 // Scanning is a performance optimization trade-off.
+
                 //TODO use termsEnum.docFreq() as heuristic
-                bool scan = cell.Level >= outerInstance.prefixGridScanLevel;
-                //simple heuristic
+                bool scan = cell.Level >= ((AbstractVisitingPrefixTreeFilter)outerInstance).prefixGridScanLevel;//simple heuristic
+
                 if (!scan)
                 {
                     //Divide & conquer (ultimately termsEnum.seek())
+
                     IEnumerator<Cell> subCellsIter = FindSubCellsToVisit(cell);
                     if (!subCellsIter.MoveNext())
                     {
-                        //not expected
-                        return;
+                        return;//not expected
                     }
-                    curVNode.children = new VNodeCellIterator
-                        (this, subCellsIter, new VNode(curVNode));
+                    curVNode.children = new VNodeCellIterator(this, subCellsIter, new VNode(curVNode));
                 }
                 else
                 {
                     //Scan (loop of termsEnum.next())
+
                     Scan(outerInstance.detailLevel);
                 }
             }
@@ -402,15 +370,13 @@ namespace Lucene.Net.Spatial.Prefix
             #region Nested type: VNodeCellIterator
 
             /// <summary>
-            /// Used for
-            /// <see cref="VNode.children">VNode.children</see>
-            /// .
+            /// Used for <see cref="VNode.children">VNode.children</see>.
             /// </summary>
             private class VNodeCellIterator : IEnumerator<VNode>
             {
                 private readonly VisitorTemplate outerInstance;
-                internal readonly IEnumerator<Cell> cellIter;
 
+                internal readonly IEnumerator<Cell> cellIter;
                 private readonly VNode vNode;
                 private bool first = true;
 
@@ -486,12 +452,10 @@ namespace Lucene.Net.Spatial.Prefix
             #endregion
 
             /// <summary>Called first to setup things.</summary>
-            /// <remarks>Called first to setup things.</remarks>
             /// <exception cref="System.IO.IOException"></exception>
             protected internal abstract void Start();
 
             /// <summary>Called last to return the result.</summary>
-            /// <remarks>Called last to return the result.</remarks>
             /// <exception cref="System.IO.IOException"></exception>
             protected internal abstract DocIdSet Finish();
 
@@ -526,12 +490,10 @@ namespace Lucene.Net.Spatial.Prefix
             /// <exception cref="System.IO.IOException"></exception>
             protected internal abstract void VisitScanned(Cell cell);
 
-            /// <exception cref="System.IO.IOException"></exception>
             protected internal virtual void PreSiblings(VNode vNode)
             {
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             protected internal virtual void PostSiblings(VNode vNode)
             {
             }
@@ -539,5 +501,43 @@ namespace Lucene.Net.Spatial.Prefix
         }
 
         #endregion
+
+        #region Nested type: VNode
+
+        /// <summary>
+        /// A Visitor Cell/Cell found via the query shape for
+        /// <see cref="VisitorTemplate">VisitorTemplate</see>
+        /// .
+        /// Sometimes these are reset(cell). It's like a LinkedList node but forms a
+        /// tree.
+        /// </summary>
+        /// <lucene.internal></lucene.internal>
+        public class VNode
+        {
+            //Note: The VNode tree adds more code to debug/maintain v.s. a flattened
+            // LinkedList that we used to have. There is more opportunity here for
+            // custom behavior (see preSiblings & postSiblings) but that's not
+            // leveraged yet. Maybe this is slightly more GC friendly.
+
+            internal readonly VNode parent;//only null at the root
+            internal IEnumerator<VNode> children;//null, then sometimes set, then null
+            internal Cell cell;//not null (except initially before reset())
+
+            /// <summary>call reset(cell) after to set the cell.</summary>
+            internal VNode(VNode parent)
+            {
+                // remember to call reset(cell) after
+                this.parent = parent;
+            }
+
+            internal virtual void Reset(Cell cell)
+            {
+                Debug.Assert(cell != null);
+                this.cell = cell;
+                Debug.Assert(children == null);
+            }
+        }
+
+        #endregion
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/ContainsPrefixTreeFilter.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/ContainsPrefixTreeFilter.cs b/src/Lucene.Net.Spatial/Prefix/ContainsPrefixTreeFilter.cs
index ca0eb3d..b36604e 100644
--- a/src/Lucene.Net.Spatial/Prefix/ContainsPrefixTreeFilter.cs
+++ b/src/Lucene.Net.Spatial/Prefix/ContainsPrefixTreeFilter.cs
@@ -38,6 +38,16 @@ namespace Lucene.Net.Spatial.Prefix
     /// <lucene.experimental></lucene.experimental>
     public class ContainsPrefixTreeFilter : AbstractPrefixTreeFilter
     {
+        // Future optimizations:
+        //   Instead of seekExact, use seekCeil with some leap-frogging, like Intersects does.
+
+        /// <summary>
+        /// If the spatial data for a document is comprised of multiple overlapping or adjacent parts,
+        /// it might fail to match a query shape when doing the CONTAINS predicate when the sum of
+        /// those shapes contain the query shape but none do individually. Set this to false to
+        /// increase performance if you don't care about that circumstance (such as if your indexed
+        /// data doesn't even have such conditions).  See LUCENE-5062.
+        /// </summary>
         protected readonly bool multiOverlappingIndexedShapes;
 
         public ContainsPrefixTreeFilter(IShape queryShape, string fieldName, SpatialPrefixTree grid, int detailLevel, bool multiOverlappingIndexedShapes)
@@ -46,7 +56,18 @@ namespace Lucene.Net.Spatial.Prefix
             this.multiOverlappingIndexedShapes = multiOverlappingIndexedShapes;
         }
 
-        /// <exception cref="System.IO.IOException"></exception>
+        public override bool Equals(object o)
+        {
+            if (!base.Equals(o))
+                return false;
+            return multiOverlappingIndexedShapes == ((ContainsPrefixTreeFilter)o).multiOverlappingIndexedShapes;
+        }
+
+        public override int GetHashCode()
+        {
+            return base.GetHashCode() + (multiOverlappingIndexedShapes ? 1 : 0);
+        }
+
         public override DocIdSet GetDocIdSet(AtomicReaderContext context, Bits acceptDocs)
         {
             return new ContainsVisitor(this, context, acceptDocs).Visit(grid.WorldCell, acceptDocs);
@@ -54,23 +75,12 @@ namespace Lucene.Net.Spatial.Prefix
 
         private class ContainsVisitor : BaseTermsEnumTraverser
         {
-            private readonly IShape queryShape;
-            private readonly int detailLevel;
-            private readonly bool multiOverlappingIndexedShapes;
-            private SpatialPrefixTree grid;
-
-            /// <exception cref="System.IO.IOException"></exception>
             public ContainsVisitor(ContainsPrefixTreeFilter outerInstance, AtomicReaderContext context, Bits acceptDocs)
                 : base(outerInstance, context, acceptDocs)
             {
-                this.queryShape = outerInstance.queryShape;
-                this.detailLevel = outerInstance.detailLevel;
-                this.grid = outerInstance.grid;
-                this.multiOverlappingIndexedShapes = outerInstance.multiOverlappingIndexedShapes;
             }
 
             internal BytesRef termBytes = new BytesRef();
-
             internal Cell nextCell;//see getLeafDocs
 
             /// <remarks>This is the primary algorithm; recursive.  Returns null if finds none.</remarks>
@@ -82,17 +92,19 @@ namespace Lucene.Net.Spatial.Prefix
                     //signals all done
                     return null;
                 }
+
+                ContainsPrefixTreeFilter outerInstance = (ContainsPrefixTreeFilter)base.outerInstance;
+
                 //Leaf docs match all query shape
                 SmallDocSet leafDocs = GetLeafDocs(cell, acceptContains);
-                // Get the AND of all child results
+                // Get the AND of all child results (into combinedSubResults)
                 SmallDocSet combinedSubResults = null;
-
                 //   Optimization: use null subCellsFilter when we know cell is within the query shape.
-                IShape subCellsFilter = queryShape;
-                if (cell.Level != 0 && ((cell.GetShapeRel() == null || cell.GetShapeRel() == SpatialRelation.WITHIN)))
+                IShape subCellsFilter = outerInstance.queryShape;
+                if (cell.Level != 0 && ((cell.GetShapeRel() == SpatialRelation.NULL_VALUE || cell.GetShapeRel() == SpatialRelation.WITHIN)))
                 {
                     subCellsFilter = null;
-                    System.Diagnostics.Debug.Assert(cell.GetShape().Relate(queryShape) == SpatialRelation.WITHIN);
+                    Debug.Assert(cell.GetShape().Relate(outerInstance.queryShape) == SpatialRelation.WITHIN);
                 }
                 ICollection<Cell> subCells = cell.GetSubCells(subCellsFilter);
                 foreach (Cell subCell in subCells)
@@ -101,14 +113,14 @@ namespace Lucene.Net.Spatial.Prefix
                     {
                         combinedSubResults = null;
                     }
-                    else if (subCell.Level == detailLevel)
+                    else if (subCell.Level == outerInstance.detailLevel)
                     {
                         combinedSubResults = GetDocs(subCell, acceptContains);
                     }
-                    else if (!multiOverlappingIndexedShapes && 
+                    else if (!outerInstance.multiOverlappingIndexedShapes && 
                         subCell.GetShapeRel() == SpatialRelation.WITHIN)
                     {
-                        combinedSubResults = GetLeafDocs(subCell, acceptContains);
+                        combinedSubResults = GetLeafDocs(subCell, acceptContains); //recursion
                     }
                     else
                     {
@@ -130,15 +142,14 @@ namespace Lucene.Net.Spatial.Prefix
                     {
                         return combinedSubResults;
                     }
-                    return leafDocs.Union(combinedSubResults);
+                    return leafDocs.Union(combinedSubResults);//union is 'or'
                 }
                 return leafDocs;
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             private bool SeekExact(Cell cell)
             {
-                System.Diagnostics.Debug.Assert(new BytesRef(cell.GetTokenBytes()).CompareTo(termBytes) > 0);
+                Debug.Assert(new BytesRef(cell.GetTokenBytes()).CompareTo(termBytes) > 0);
                 this.termBytes.Bytes = cell.GetTokenBytes();
                 this.termBytes.Length = this.termBytes.Bytes.Length;
                 if (termsEnum == null)
@@ -146,16 +157,14 @@ namespace Lucene.Net.Spatial.Prefix
                 return this.termsEnum.SeekExact(termBytes);
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             private SmallDocSet GetDocs(Cell cell, Bits acceptContains)
             {
-                System.Diagnostics.Debug.Assert(new BytesRef(cell.GetTokenBytes()).Equals(termBytes));
+                Debug.Assert(new BytesRef(cell.GetTokenBytes()).Equals(termBytes));
                 return this.CollectDocs(acceptContains);
             }
 
             private Cell lastLeaf = null;//just for assertion
 
-            /// <exception cref="System.IO.IOException"></exception>
             private SmallDocSet GetLeafDocs(Cell leafCell, Bits acceptContains)
             {
                 Debug.Assert(new BytesRef(leafCell.GetTokenBytes()).Equals(termBytes));
@@ -167,11 +176,10 @@ namespace Lucene.Net.Spatial.Prefix
                 BytesRef nextTerm = this.termsEnum.Next();
                 if (nextTerm == null)
                 {
-                    termsEnum = null;
-                    //signals all done
+                    termsEnum = null;//signals all done
                     return null;
                 }
-                nextCell = grid.GetCell(nextTerm.Bytes, nextTerm.Offset, nextTerm.Length, this.nextCell);
+                nextCell = outerInstance.grid.GetCell(nextTerm.Bytes, nextTerm.Offset, nextTerm.Length, this.nextCell);
                 if (nextCell.Level == leafCell.Level && nextCell.IsLeaf())
                 {
                     return CollectDocs(acceptContains);
@@ -182,10 +190,10 @@ namespace Lucene.Net.Spatial.Prefix
                 }
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             private SmallDocSet CollectDocs(Bits acceptContains)
             {
                 SmallDocSet set = null;
+
                 docsEnum = termsEnum.Docs(acceptContains, docsEnum, DocsEnum.FLAG_NONE);
                 int docid;
                 while ((docid = docsEnum.NextDoc()) != DocIdSetIterator.NO_MORE_DOCS)
@@ -213,7 +221,6 @@ namespace Lucene.Net.Spatial.Prefix
         private class SmallDocSet : DocIdSet, Bits
         {
             private readonly SentinelIntSet intSet;
-
             private int maxInt = 0;
 
             public SmallDocSet(int size)
@@ -221,7 +228,7 @@ namespace Lucene.Net.Spatial.Prefix
                 intSet = new SentinelIntSet(size, -1);
             }
 
-            public bool Get(int index)
+            public virtual bool Get(int index)
             {
                 return intSet.Exists(index);
             }
@@ -235,7 +242,8 @@ namespace Lucene.Net.Spatial.Prefix
                 }
             }
 
-            int Bits.Length()
+            /// <summary>Largest docid.</summary>
+            public virtual int Length()
             {
                 return maxInt;
             }
@@ -290,10 +298,9 @@ namespace Lucene.Net.Spatial.Prefix
                 {
                     this.size = size;
                     this.docs = docs;
-                    this.idx = -1;
                 }
 
-                internal int idx;
+                internal int idx = -1;
 
                 public override int DocID()
                 {
@@ -307,7 +314,6 @@ namespace Lucene.Net.Spatial.Prefix
                     }
                 }
 
-                /// <exception cref="System.IO.IOException"></exception>
                 public override int NextDoc()
                 {
                     if (++idx < size)
@@ -317,7 +323,6 @@ namespace Lucene.Net.Spatial.Prefix
                     return NO_MORE_DOCS;
                 }
 
-                /// <exception cref="System.IO.IOException"></exception>
                 public override int Advance(int target)
                 {
                     //for this small set this is likely faster vs. a binary search
@@ -348,7 +353,7 @@ namespace Lucene.Net.Spatial.Prefix
                     }
                     docs[d++] = v;
                 }
-                System.Diagnostics.Debug.Assert(d == intSet.Size());
+                Debug.Assert(d == intSet.Size());
                 int size = d;
                 //sort them
                 Array.Sort(docs, 0, size);

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/IntersectsPrefixTreeFilter.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/IntersectsPrefixTreeFilter.cs b/src/Lucene.Net.Spatial/Prefix/IntersectsPrefixTreeFilter.cs
index 3d378e0..a150e37 100644
--- a/src/Lucene.Net.Spatial/Prefix/IntersectsPrefixTreeFilter.cs
+++ b/src/Lucene.Net.Spatial/Prefix/IntersectsPrefixTreeFilter.cs
@@ -75,7 +75,6 @@ namespace Lucene.Net.Spatial.Prefix
                 return results;
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             protected internal override bool Visit(Cell cell)
             {
                 if (cell.GetShapeRel() == SpatialRelation.WITHIN || cell.Level == outerInstance.detailLevel)
@@ -86,13 +85,11 @@ namespace Lucene.Net.Spatial.Prefix
                 return true;
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             protected internal override void VisitLeaf(Cell cell)
             {
                 CollectDocs(results);
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             protected internal override void VisitScanned(Cell cell)
             {
                 if (outerInstance.queryShape.Relate(cell.GetShape()).Intersects())

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs b/src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs
index ea36656..a10e717 100644
--- a/src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs
+++ b/src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs
@@ -97,7 +97,7 @@ namespace Lucene.Net.Spatial.Prefix
 
         protected internal readonly bool simplifyIndexedCells;
         protected internal int defaultFieldValuesArrayLen = 2;
-        protected internal double distErrPct = SpatialArgs.DEFAULT_DISTERRPCT;
+        protected internal double distErrPct = SpatialArgs.DEFAULT_DISTERRPCT;// [ 0 TO 0.5 ]
 
         public PrefixTreeStrategy(SpatialPrefixTree grid, string fieldName, bool simplifyIndexedCells)
             : base(grid.SpatialContext, fieldName)
@@ -155,7 +155,7 @@ namespace Lucene.Net.Spatial.Prefix
         public virtual Field[] CreateIndexableFields(IShape shape, double distErr)
         {
             int detailLevel = grid.GetLevelForDistance(distErr);
-            IList<Cell> cells = grid.GetCells(shape, detailLevel, true, simplifyIndexedCells);
+            IList<Cell> cells = grid.GetCells(shape, detailLevel, true, simplifyIndexedCells);//intermediates cells
 
             //TODO is CellTokenStream supposed to be re-used somehow? see Uwe's comments:
             //  http://code.google.com/p/lucene-spatial-playground/issues/detail?id=4
@@ -175,10 +175,7 @@ namespace Lucene.Net.Spatial.Prefix
             FieldType.Freeze();
         }
 
-        /// <summary>Outputs the tokenString of a cell, and if its a leaf, outputs it again with the leaf byte.
-        /// 	</summary>
-        /// <remarks>Outputs the tokenString of a cell, and if its a leaf, outputs it again with the leaf byte.
-        /// 	</remarks>
+        /// <summary>Outputs the tokenString of a cell, and if its a leaf, outputs it again with the leaf byte.</summary>
         internal sealed class CellTokenStream : TokenStream
         {
             private readonly ICharTermAttribute termAtt;
@@ -191,7 +188,7 @@ namespace Lucene.Net.Spatial.Prefix
                 termAtt = AddAttribute<ICharTermAttribute>();
             }
 
-            internal string nextTokenStringNeedingLeaf;
+            internal string nextTokenStringNeedingLeaf = null;
 
             public override bool IncrementToken()
             {

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/RecursivePrefixTreeStrategy.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/RecursivePrefixTreeStrategy.cs b/src/Lucene.Net.Spatial/Prefix/RecursivePrefixTreeStrategy.cs
index 37b4d41..41ae1d0 100644
--- a/src/Lucene.Net.Spatial/Prefix/RecursivePrefixTreeStrategy.cs
+++ b/src/Lucene.Net.Spatial/Prefix/RecursivePrefixTreeStrategy.cs
@@ -89,8 +89,9 @@ namespace Lucene.Net.Spatial.Prefix
             }
             else if (op == SpatialOperation.IsWithin)
             {
-                return new WithinPrefixTreeFilter(shape, FieldName, grid, detailLevel, prefixGridScanLevel
-                    , -1); //-1 flag is slower but ensures correct results
+                return new WithinPrefixTreeFilter(
+                    shape, FieldName, grid, detailLevel, prefixGridScanLevel, 
+                    -1); //-1 flag is slower but ensures correct results
             }
             else if (op == SpatialOperation.Contains)
             {

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/TermQueryPrefixTreeStrategy.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/TermQueryPrefixTreeStrategy.cs b/src/Lucene.Net.Spatial/Prefix/TermQueryPrefixTreeStrategy.cs
index d4b5d25..8017eb3 100644
--- a/src/Lucene.Net.Spatial/Prefix/TermQueryPrefixTreeStrategy.cs
+++ b/src/Lucene.Net.Spatial/Prefix/TermQueryPrefixTreeStrategy.cs
@@ -44,11 +44,10 @@ namespace Lucene.Net.Spatial.Prefix
     public class TermQueryPrefixTreeStrategy : PrefixTreeStrategy
     {
         public TermQueryPrefixTreeStrategy(SpatialPrefixTree grid, string fieldName)
-            : base(grid, fieldName, false)
+            : base(grid, fieldName, false)//do not simplify indexed cells
         {
         }
 
-        //do not simplify indexed cells
         public override Filter MakeFilter(SpatialArgs args)
         {
             SpatialOperation op = args.Operation;
@@ -58,9 +57,7 @@ namespace Lucene.Net.Spatial.Prefix
             }
             IShape shape = args.Shape;
             int detailLevel = grid.GetLevelForDistance(args.ResolveDistErr(ctx, distErrPct));
-            IList<Cell> cells = grid.GetCells(shape, detailLevel, false, true);
-            //no parents
-            //simplify
+            IList<Cell> cells = grid.GetCells(shape, detailLevel, false /*no parents*/, true /*simplify*/);
             var terms = new BytesRef[cells.Count];
             int i = 0;
             foreach (Cell cell in cells)

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/Tree/Cell.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/Tree/Cell.cs b/src/Lucene.Net.Spatial/Prefix/Tree/Cell.cs
index 8af4cd8..5969a08 100644
--- a/src/Lucene.Net.Spatial/Prefix/Tree/Cell.cs
+++ b/src/Lucene.Net.Spatial/Prefix/Tree/Cell.cs
@@ -60,10 +60,6 @@ namespace Lucene.Net.Spatial.Prefix.Tree
         /// When set via getSubCells(filter), it is the relationship between this cell
         /// and the given shape filter.
         /// </summary>
-        /// <remarks>
-        /// When set via getSubCells(filter), it is the relationship between this cell
-        /// and the given shape filter.
-        /// </remarks>
         protected internal SpatialRelation shapeRel = SpatialRelation.NULL_VALUE;//set in getSubCells(filter), and via setLeaf().
 
         /// <summary>Always false for points.</summary>
@@ -85,7 +81,7 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             this.token = token;
             if (token.Length > 0 && token[token.Length - 1] == (char)LEAF_BYTE)
             {
-                this.token = token.Substring(0, token.Length - 1);
+                this.token = token.Substring(0, (token.Length - 1) - 0);
                 SetLeaf();
             }
             if (Level == 0)
@@ -107,15 +103,6 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             B_fixLeaf();
         }
 
-        #region IComparable<Cell> Members
-
-        public virtual int CompareTo(Cell o)
-        {
-            return string.CompareOrdinal(TokenString, o.TokenString);
-        }
-
-        #endregion
-
         public virtual void Reset(byte[] bytes, int off, int len)
         {
             Debug.Assert(Level != 0);
@@ -127,23 +114,6 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             B_fixLeaf();
         }
 
-        public virtual void Reset(string token)
-        {
-            Debug.Assert(Level != 0);
-            this.token = token;
-            shapeRel = SpatialRelation.NULL_VALUE;
-
-            //converting string t0 byte[]
-            //bytes = Encoding.UTF8.GetBytes(token);
-            BytesRef utf8Result = new BytesRef(token.Length);
-            UnicodeUtil.UTF16toUTF8(token.ToCharArray(), 0, token.Length, utf8Result);
-            bytes = utf8Result.Bytes;
-
-            b_off = 0;
-            b_len = bytes.Length;
-            B_fixLeaf();
-        }
-
         private void B_fixLeaf()
         {
             //note that non-point shapes always have the maxLevels cell set with setLeaf
@@ -181,9 +151,9 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             leaf = true;
         }
 
-        /*
-         * Note: doesn't contain a trailing leaf byte.
-         */
+        /// <summary>
+        /// Note: doesn't contain a trailing leaf byte.
+        /// </summary>
         public virtual String TokenString
         {
             get
@@ -195,7 +165,6 @@ namespace Lucene.Net.Spatial.Prefix.Tree
         }
 
         /// <summary>Note: doesn't contain a trailing leaf byte.</summary>
-        /// <remarks>Note: doesn't contain a trailing leaf byte.</remarks>
         public virtual byte[] GetTokenBytes()
         {
             if (bytes != null)
@@ -207,11 +176,7 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             }
             else
             {
-                //converting string t0 byte[]
-                //bytes = Encoding.UTF8.GetBytes(token);
-                BytesRef utf8Result = new BytesRef(token.Length);
-                UnicodeUtil.UTF16toUTF8(token.ToCharArray(), 0, token.Length, utf8Result);
-                bytes = utf8Result.Bytes;
+                bytes = Encoding.UTF8.GetBytes(token);
                 b_off = 0;
                 b_len = bytes.Length;
             }
@@ -323,6 +288,15 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             return GetShape().Center;
         }
 
+        #region IComparable<Cell> Members
+
+        public virtual int CompareTo(Cell o)
+        {
+            return string.CompareOrdinal(TokenString, o.TokenString);
+        }
+
+        #endregion
+
         #region Equality overrides
 
         public override bool Equals(object obj)

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/Tree/GeohashPrefixTree.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/Tree/GeohashPrefixTree.cs b/src/Lucene.Net.Spatial/Prefix/Tree/GeohashPrefixTree.cs
index 2218ca0..2c775c8 100644
--- a/src/Lucene.Net.Spatial/Prefix/Tree/GeohashPrefixTree.cs
+++ b/src/Lucene.Net.Spatial/Prefix/Tree/GeohashPrefixTree.cs
@@ -35,27 +35,45 @@ namespace Lucene.Net.Spatial.Prefix.Tree
     /// <lucene.experimental></lucene.experimental>
     public class GeohashPrefixTree : SpatialPrefixTree
     {
+        #region Nested type: Factory
+
+        /// <summary>
+        /// Factory for creating
+        /// <see cref="GeohashPrefixTree">GeohashPrefixTree</see>
+        /// instances with useful defaults
+        /// </summary>
+        public class Factory : SpatialPrefixTreeFactory
+        {
+            protected internal override int GetLevelForDistance(double degrees)
+            {
+                var grid = new GeohashPrefixTree(ctx, GeohashPrefixTree.MaxLevelsPossible);
+                return grid.GetLevelForDistance(degrees);
+            }
+
+            protected internal override SpatialPrefixTree NewSPT()
+            {
+                return new GeohashPrefixTree(ctx, maxLevels.HasValue ? maxLevels.Value : GeohashPrefixTree.MaxLevelsPossible);
+            }
+        }
+
+        #endregion
+
         public GeohashPrefixTree(SpatialContext ctx, int maxLevels)
             : base(ctx, maxLevels)
         {
             IRectangle bounds = ctx.WorldBounds;
             if (bounds.MinX != -180)
             {
-                throw new ArgumentException("Geohash only supports lat-lon world bounds. Got " +
-                                            bounds);
+                throw new ArgumentException("Geohash only supports lat-lon world bounds. Got " + bounds);
             }
             int Maxp = MaxLevelsPossible;
             if (maxLevels <= 0 || maxLevels > Maxp)
             {
-                throw new ArgumentException("maxLen must be [1-" + Maxp + "] but got " + maxLevels
-                    );
+                throw new ArgumentException("maxLen must be [1-" + Maxp + "] but got " + maxLevels);
             }
         }
 
-        /// <summary>Any more than this and there's no point (double lat & lon are the same).
-        /// 	</summary>
-        /// <remarks>Any more than this and there's no point (double lat & lon are the same).
-        /// 	</remarks>
+        /// <summary>Any more than this and there's no point (double lat & lon are the same).</summary>
         public static int MaxLevelsPossible
         {
             get { return GeohashUtils.MAX_PRECISION; }
@@ -65,9 +83,9 @@ namespace Lucene.Net.Spatial.Prefix.Tree
         {
             if (dist == 0)
             {
-                return maxLevels;
+                return maxLevels;//short circuit
             }
-            //short circuit
+            
             int level = GeohashUtils.LookupHashLenForWidthHeight(dist, dist);
             return Math.Max(Math.Min(level, maxLevels), 1);
         }
@@ -88,35 +106,10 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             return new GhCell(this, bytes, offset, len);
         }
 
-        #region Nested type: Factory
-
-        /// <summary>
-        /// Factory for creating
-        /// <see cref="GeohashPrefixTree">GeohashPrefixTree</see>
-        /// instances with useful defaults
-        /// </summary>
-        public class Factory : SpatialPrefixTreeFactory
-        {
-            protected internal override int GetLevelForDistance(double degrees)
-            {
-                var grid = new GeohashPrefixTree(ctx, MaxLevelsPossible);
-                return grid.GetLevelForDistance(degrees);
-            }
-
-            protected internal override SpatialPrefixTree NewSPT()
-            {
-                return new GeohashPrefixTree(ctx, maxLevels.HasValue ? maxLevels.Value : MaxLevelsPossible);
-            }
-        }
-
-        #endregion
-
         #region Nested type: GhCell
 
         internal class GhCell : Cell
         {
-            private IShape shape;
-
             internal GhCell(GeohashPrefixTree outerInstance, string token)
                 : base(outerInstance, token)
             {
@@ -135,8 +128,7 @@ namespace Lucene.Net.Spatial.Prefix.Tree
 
             protected internal override ICollection<Cell> GetSubCells()
             {
-                string[] hashes = GeohashUtils.GetSubGeohashes(Geohash);
-                //sorted
+                string[] hashes = GeohashUtils.GetSubGeohashes(Geohash);//sorted
                 IList<Cell> cells = new List<Cell>(hashes.Length);
                 foreach (string hash in hashes)
                 {
@@ -147,17 +139,16 @@ namespace Lucene.Net.Spatial.Prefix.Tree
 
             public override int GetSubCellsSize()
             {
-                return 32;
+                return 32;//8x4
             }
 
-            //8x4
             public override Cell GetSubCell(IPoint p)
             {
-                return outerInstance.GetCell(p, Level + 1);
+                return outerInstance.GetCell(p, Level + 1);//not performant!
             }
 
-            //not performant!
-            //cache
+            private IShape shape;//cache
+
             public override IShape GetShape()
             {
                 if (shape == null)

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/Tree/QuadPrefixTree.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/Tree/QuadPrefixTree.cs b/src/Lucene.Net.Spatial/Prefix/Tree/QuadPrefixTree.cs
index 26763cc..ce6f48b 100644
--- a/src/Lucene.Net.Spatial/Prefix/Tree/QuadPrefixTree.cs
+++ b/src/Lucene.Net.Spatial/Prefix/Tree/QuadPrefixTree.cs
@@ -34,47 +34,69 @@ namespace Lucene.Net.Spatial.Prefix.Tree
     /// <lucene.experimental></lucene.experimental>
     public class QuadPrefixTree : SpatialPrefixTree
     {
-        public const int MaxLevelsPossible = 50;
+        #region Nested type: Factory
 
-        public const int DefaultMaxLevels = 12;
+        /// <summary>
+        /// Factory for creating
+        /// <see cref="QuadPrefixTree">QuadPrefixTree</see>
+        /// instances with useful defaults
+        /// </summary>
+        public class Factory : SpatialPrefixTreeFactory
+        {
+            protected internal override int GetLevelForDistance(double degrees)
+            {
+                var grid = new QuadPrefixTree(ctx, MaxLevelsPossible);
+                return grid.GetLevelForDistance(degrees);
+            }
 
-        public readonly double gridH;
-        private readonly double gridW;
+            protected internal override SpatialPrefixTree NewSPT()
+            {
+                return new QuadPrefixTree(ctx, maxLevels.HasValue ? maxLevels.Value : MaxLevelsPossible);
+            }
+        }
 
-        internal readonly double[] levelH;
+        #endregion
 
-        internal readonly int[] levelN;
-        internal readonly int[] levelS;
-        internal readonly double[] levelW;
-        private readonly double xmax;
-        private readonly double xmid;
+        public const int MaxLevelsPossible = 50;//not really sure how big this should be
+
+        public const int DefaultMaxLevels = 12;
         private readonly double xmin;
+        private readonly double xmax;
+        private readonly double ymin;
         private readonly double ymax;
+        private readonly double xmid;
         private readonly double ymid;
-        private readonly double ymin;
+
+        private readonly double gridW;
+        public readonly double gridH;
+        
+        internal readonly double[] levelW;
+        internal readonly double[] levelH;
+        internal readonly int[] levelS; // side
+        internal readonly int[] levelN; // number
 
         public QuadPrefixTree(SpatialContext ctx, IRectangle bounds, int maxLevels)
             : base(ctx, maxLevels)
         {
-            //not really sure how big this should be
-            // side
-            // number
             xmin = bounds.MinX;
             xmax = bounds.MaxX;
             ymin = bounds.MinY;
             ymax = bounds.MaxY;
+
             levelW = new double[maxLevels];
             levelH = new double[maxLevels];
             levelS = new int[maxLevels];
             levelN = new int[maxLevels];
+
             gridW = xmax - xmin;
             gridH = ymax - ymin;
-            xmid = xmin + gridW / 2.0;
-            ymid = ymin + gridH / 2.0;
+            this.xmid = xmin + gridW / 2.0;
+            this.ymid = ymin + gridH / 2.0;
             levelW[0] = gridW / 2.0;
             levelH[0] = gridH / 2.0;
             levelS[0] = 2;
             levelN[0] = 4;
+
             for (int i = 1; i < levelW.Length; i++)
             {
                 levelW[i] = levelW[i - 1] / 2.0;
@@ -111,9 +133,8 @@ namespace Lucene.Net.Spatial.Prefix.Tree
 
         public override int GetLevelForDistance(double dist)
         {
-            if (dist == 0)
+            if (dist == 0)//short circuit
             {
-                //short circuit
                 return maxLevels;
             }
             for (int i = 0; i < maxLevels - 1; i++)
@@ -145,12 +166,19 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             return new QuadCell(this, bytes, offset, len);
         }
 
-        private void Build(double x, double y, int level, IList<Cell> matches, StringBuilder
-                                                                                   str, IShape shape, int maxLevel)
+        private void Build(
+            double x, 
+            double y, 
+            int level, 
+            IList<Cell> matches, 
+            StringBuilder str, 
+            IShape shape, 
+            int maxLevel)
         {
             Debug.Assert(str.Length == level);
             double w = levelW[level] / 2;
             double h = levelH[level] / 2;
+
             // Z-Order
             // http://en.wikipedia.org/wiki/Z-order_%28curve%29
             CheckBattenberg('A', x - w, y + h, level, matches, str, shape, maxLevel);
@@ -163,13 +191,20 @@ namespace Lucene.Net.Spatial.Prefix.Tree
         // http://en.wikipedia.org/wiki/Hilbert_curve
         // http://blog.notdot.net/2009/11/Damn-Cool-Algorithms-Spatial-indexing-with-Quadtrees-and-Hilbert-Curves
         // if we actually use the range property in the query, this could be useful
-        private void CheckBattenberg(char c, double cx, double cy, int level, IList<Cell>
-                                                                                  matches, StringBuilder str,
-                                     IShape shape, int maxLevel)
+        private void CheckBattenberg(
+            char c, 
+            double cx, 
+            double cy, 
+            int level, 
+            IList<Cell> matches, 
+            StringBuilder str,
+            IShape shape, 
+            int maxLevel)
         {
             Debug.Assert(str.Length == level);
             double w = levelW[level] / 2;
             double h = levelH[level] / 2;
+
             int strlen = str.Length;
             IRectangle rectangle = ctx.MakeRectangle(cx - w, cx + w, cy - h, cy + h);
             SpatialRelation v = shape.Relate(rectangle);
@@ -179,67 +214,38 @@ namespace Lucene.Net.Spatial.Prefix.Tree
                 //str.append(SpatialPrefixGrid.COVER);
                 matches.Add(new QuadCell(this, str.ToString(), v.Transpose()));
             }
-            else
+            else if (SpatialRelation.DISJOINT == v)
             {
-                if (SpatialRelation.DISJOINT == v)
+                // nothing
+            }
+            else // SpatialRelation.WITHIN, SpatialRelation.INTERSECTS
+            {
+                str.Append(c);
+
+                int nextLevel = level + 1;
+                if (nextLevel >= maxLevel)
                 {
+                    //str.append(SpatialPrefixGrid.INTERSECTS);
+                    matches.Add(new QuadCell(this, str.ToString(), v.Transpose()));
                 }
                 else
                 {
-                    // nothing
-                    // SpatialRelation.WITHIN, SpatialRelation.INTERSECTS
-                    str.Append(c);
-                    int nextLevel = level + 1;
-                    if (nextLevel >= maxLevel)
-                    {
-                        //str.append(SpatialPrefixGrid.INTERSECTS);
-                        matches.Add(new QuadCell(this, str.ToString(), v.Transpose()));
-                    }
-                    else
-                    {
-                        Build(cx, cy, nextLevel, matches, str, shape, maxLevel);
-                    }
+                    Build(cx, cy, nextLevel, matches, str, shape, maxLevel);
                 }
             }
             str.Length = strlen;
         }
 
-        #region Nested type: Factory
-
-        /// <summary>
-        /// Factory for creating
-        /// <see cref="QuadPrefixTree">QuadPrefixTree</see>
-        /// instances with useful defaults
-        /// </summary>
-        public class Factory : SpatialPrefixTreeFactory
-        {
-            protected internal override int GetLevelForDistance(double degrees)
-            {
-                var grid = new QuadPrefixTree(ctx, MaxLevelsPossible);
-                return grid.GetLevelForDistance(degrees);
-            }
-
-            protected internal override SpatialPrefixTree NewSPT()
-            {
-                return new QuadPrefixTree(ctx, maxLevels.HasValue ? maxLevels.Value : MaxLevelsPossible);
-            }
-        }
-
-        #endregion
-
         #region Nested type: QuadCell
 
         internal class QuadCell : Cell
         {
-            private IShape shape;
-
             public QuadCell(QuadPrefixTree outerInstance, string token)
                 : base(outerInstance, token)
             {
             }
 
-            public QuadCell(QuadPrefixTree outerInstance, string token, SpatialRelation shapeRel
-                )
+            public QuadCell(QuadPrefixTree outerInstance, string token, SpatialRelation shapeRel)
                 : base(outerInstance, token)
             {
                 this.shapeRel = shapeRel;
@@ -274,11 +280,11 @@ namespace Lucene.Net.Spatial.Prefix.Tree
 
             public override Cell GetSubCell(IPoint p)
             {
-                return outerInstance.GetCell(p, Level + 1);
+                return outerInstance.GetCell(p, Level + 1);//not performant!
             }
 
-            //not performant!
-            //cache
+            private IShape shape; //cache
+
             public override IShape GetShape()
             {
                 if (shape == null)
@@ -301,31 +307,22 @@ namespace Lucene.Net.Spatial.Prefix.Tree
                     {
                         ymin += outerInstance.levelH[i];
                     }
+                    else if ('B' == c || 'b' == c)
+                    {
+                        xmin += outerInstance.levelW[i];
+                        ymin += outerInstance.levelH[i];
+                    }
+                    else if ('C' == c || 'c' == c)
+                    {
+                        // nothing really
+                    }
+                    else if ('D' == c || 'd' == c)
+                    {
+                        xmin += outerInstance.levelW[i];
+                    }
                     else
                     {
-                        if ('B' == c || 'b' == c)
-                        {
-                            xmin += outerInstance.levelW[i];
-                            ymin += outerInstance.levelH[i];
-                        }
-                        else
-                        {
-                            if ('C' == c || 'c' == c)
-                            {
-                            }
-                            else
-                            {
-                                // nothing really
-                                if ('D' == c || 'd' == c)
-                                {
-                                    xmin += outerInstance.levelW[i];
-                                }
-                                else
-                                {
-                                    throw new Exception("unexpected char: " + c);
-                                }
-                            }
-                        }
+                        throw new Exception("unexpected char: " + c);
                     }
                 }
                 int len = token.Length;
@@ -343,9 +340,7 @@ namespace Lucene.Net.Spatial.Prefix.Tree
                 }
                 return outerInstance.ctx.MakeRectangle(xmin, xmin + width, ymin, ymin + height);
             }
-
-            //QuadCell
-        }
+        }//QuadCell
 
         #endregion
     }

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTree.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTree.cs b/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTree.cs
index 77f61fb..9964485 100644
--- a/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTree.cs
+++ b/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTree.cs
@@ -111,10 +111,9 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             return Math.Sqrt(width * width + height * height);
         }
 
-        [System.NonSerialized]
-        private Cell worldCell;
+        [NonSerialized]
+        private Cell worldCell;//cached
 
-        //cached
         /// <summary>Returns the level 0 cell which encompasses all spatial data.</summary>
         /// <remarks>
         /// Returns the level 0 cell which encompasses all spatial data. Equivalent to
@@ -223,9 +222,8 @@ namespace Lucene.Net.Spatial.Prefix.Tree
         {
             if (cell.Level == detailLevel)
             {
-                cell.SetLeaf();
+                cell.SetLeaf();//FYI might already be a leaf
             }
-            //FYI might already be a leaf
             if (cell.IsLeaf())
             {
                 result.Add(cell);
@@ -235,6 +233,7 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             {
                 result.Add(cell);
             }
+
             ICollection<Cell> subCells = cell.GetSubCells(shape);
             int leaves = 0;
             foreach (Cell subCell in subCells)
@@ -247,20 +246,19 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             //can we simplify?
             if (simplify && leaves == cell.GetSubCellsSize() && cell.Level != 0)
             {
+                //Optimization: substitute the parent as a leaf instead of adding all
+                // children as leaves
+
+                //remove the leaves
                 do
                 {
-                    //Optimization: substitute the parent as a leaf instead of adding all
-                    // children as leaves
-                    //remove the leaves
-                    result.RemoveAt(result.Count - 1);
+                    result.RemoveAt(result.Count - 1);//remove last
                 }
                 while (--leaves > 0);
-                //remove last
                 //add cell as the leaf
                 cell.SetLeaf();
-                if (!inclParents)
+                if (!inclParents)// otherwise it was already added up above
                 {
-                    // otherwise it was already added up above
                     result.Add(cell);
                 }
                 return true;

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTreeFactory.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTreeFactory.cs b/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTreeFactory.cs
index cca47ec..ad70563 100644
--- a/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTreeFactory.cs
+++ b/src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTreeFactory.cs
@@ -31,21 +31,15 @@ namespace Lucene.Net.Spatial.Prefix.Tree
     /// <lucene.experimental></lucene.experimental>
     public abstract class SpatialPrefixTreeFactory
     {
-        private const double DefaultGeoMaxDetailKm = 0.001;
-
+        private const double DefaultGeoMaxDetailKm = 0.001;//1m
         public const string PrefixTree = "prefixTree";
-
         public const string MaxLevels = "maxLevels";
-
         public const string MaxDistErr = "maxDistErr";
 
         protected internal IDictionary<string, string> args;
-
         protected internal SpatialContext ctx;
-
         protected internal int? maxLevels;
 
-        //1m
         /// <summary>The factory  is looked up via "prefixTree" in args, expecting "geohash" or "quad".
         /// 	</summary>
         /// <remarks>
@@ -55,8 +49,8 @@ namespace Lucene.Net.Spatial.Prefix.Tree
         public static SpatialPrefixTree MakeSPT(IDictionary<string, string> args, SpatialContext ctx)
         {
             SpatialPrefixTreeFactory instance;
-            string cname = args[PrefixTree];
-            if (cname == null)
+            string cname;
+            if (!args.TryGetValue(PrefixTree, out cname))
             {
                 cname = ctx.IsGeo ? "geohash" : "quad";
             }
@@ -64,23 +58,20 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             {
                 instance = new GeohashPrefixTree.Factory();
             }
+            else if ("quad".Equals(cname, StringComparison.OrdinalIgnoreCase))
+            {
+                instance = new QuadPrefixTree.Factory();
+            }
             else
             {
-                if ("quad".Equals(cname, StringComparison.OrdinalIgnoreCase))
+                try
                 {
-                    instance = new QuadPrefixTree.Factory();
+                    Type c = Type.GetType(cname);
+                    instance = (SpatialPrefixTreeFactory)Activator.CreateInstance(c);
                 }
-                else
+                catch (Exception e)
                 {
-                    try
-                    {
-                        Type c = Type.GetType(cname);
-                        instance = (SpatialPrefixTreeFactory)Activator.CreateInstance(c);
-                    }
-                    catch (Exception e)
-                    {
-                        throw new Exception(string.Empty, e);
-                    }
+                    throw new ApplicationException(string.Empty, e);
                 }
             }
             instance.Init(args, ctx);
@@ -96,15 +87,15 @@ namespace Lucene.Net.Spatial.Prefix.Tree
 
         protected internal virtual void InitMaxLevels()
         {
-            string mlStr = args[MaxLevels];
-            if (mlStr != null)
+            string mlStr;
+            if (args.TryGetValue(MaxLevels, out mlStr))
             {
                 maxLevels = int.Parse(mlStr, CultureInfo.InvariantCulture);
                 return;
             }
             double degrees;
-            string maxDetailDistStr = args[MaxDistErr];
-            if (maxDetailDistStr == null)
+            string maxDetailDistStr;
+            if (!args.TryGetValue(MaxDistErr, out maxDetailDistStr))
             {
                 if (!ctx.IsGeo)
                 {
@@ -115,16 +106,14 @@ namespace Lucene.Net.Spatial.Prefix.Tree
             }
             else
             {
-                degrees = double.Parse(maxDetailDistStr);
+                degrees = double.Parse(maxDetailDistStr, CultureInfo.InvariantCulture);
             }
             maxLevels = GetLevelForDistance(degrees);
         }
 
         /// <summary>
         /// Calls
-        /// <see cref="SpatialPrefixTree.GetLevelForDistance(double)">SpatialPrefixTree.GetLevelForDistance(double)
-        /// 	</see>
-        /// .
+        /// <see cref="SpatialPrefixTree.GetLevelForDistance(double)">SpatialPrefixTree.GetLevelForDistance(double)</see>.
         /// </summary>
         protected internal abstract int GetLevelForDistance(double degrees);
 

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Prefix/WithinPrefixTreeFilter.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Prefix/WithinPrefixTreeFilter.cs b/src/Lucene.Net.Spatial/Prefix/WithinPrefixTreeFilter.cs
index 11cfc46..110739f 100644
--- a/src/Lucene.Net.Spatial/Prefix/WithinPrefixTreeFilter.cs
+++ b/src/Lucene.Net.Spatial/Prefix/WithinPrefixTreeFilter.cs
@@ -93,56 +93,53 @@ namespace Lucene.Net.Spatial.Prefix
             {
                 return ctx.MakeCircle((IPoint)shape, distErr);
             }
+            else if (shape is ICircle)
+            {
+                var circle = (ICircle)shape;
+                double newDist = circle.Radius + distErr;
+                if (ctx.IsGeo && newDist > 180)
+                {
+                    newDist = 180;
+                }
+                return ctx.MakeCircle(circle.Center, newDist);
+            }
             else
             {
-                if (shape is ICircle)
+                IRectangle bbox = shape.BoundingBox;
+                double newMinX = bbox.MinX - distErr;
+                double newMaxX = bbox.MaxX + distErr;
+                double newMinY = bbox.MinY - distErr;
+                double newMaxY = bbox.MaxY + distErr;
+                if (ctx.IsGeo)
                 {
-                    var circle = (ICircle)shape;
-                    double newDist = circle.Radius + distErr;
-                    if (ctx.IsGeo && newDist > 180)
+                    if (newMinY < -90)
                     {
-                        newDist = 180;
+                        newMinY = -90;
                     }
-                    return ctx.MakeCircle(circle.Center, newDist);
-                }
-                else
-                {
-                    IRectangle bbox = shape.BoundingBox;
-                    double newMinX = bbox.MinX - distErr;
-                    double newMaxX = bbox.MaxX + distErr;
-                    double newMinY = bbox.MinY - distErr;
-                    double newMaxY = bbox.MaxY + distErr;
-                    if (ctx.IsGeo)
+                    if (newMaxY > 90)
+                    {
+                        newMaxY = 90;
+                    }
+                    if (newMinY == -90 || newMaxY == 90 || bbox.Width + 2 * distErr > 360)
                     {
-                        if (newMinY < -90)
-                        {
-                            newMinY = -90;
-                        }
-                        if (newMaxY > 90)
-                        {
-                            newMaxY = 90;
-                        }
-                        if (newMinY == -90 || newMaxY == 90 || bbox.Width + 2 * distErr > 360)
-                        {
-                            newMinX = -180;
-                            newMaxX = 180;
-                        }
-                        else
-                        {
-                            newMinX = DistanceUtils.NormLonDEG(newMinX);
-                            newMaxX = DistanceUtils.NormLonDEG(newMaxX);
-                        }
+                        newMinX = -180;
+                        newMaxX = 180;
                     }
                     else
                     {
-                        //restrict to world bounds
-                        newMinX = Math.Max(newMinX, ctx.WorldBounds.MinX);
-                        newMaxX = Math.Min(newMaxX, ctx.WorldBounds.MaxX);
-                        newMinY = Math.Max(newMinY, ctx.WorldBounds.MinY);
-                        newMaxY = Math.Min(newMaxY, ctx.WorldBounds.MaxY);
+                        newMinX = DistanceUtils.NormLonDEG(newMinX);
+                        newMaxX = DistanceUtils.NormLonDEG(newMaxX);
                     }
-                    return ctx.MakeRectangle(newMinX, newMaxX, newMinY, newMaxY);
                 }
+                else
+                {
+                    //restrict to world bounds
+                    newMinX = Math.Max(newMinX, ctx.WorldBounds.MinX);
+                    newMaxX = Math.Min(newMaxX, ctx.WorldBounds.MaxX);
+                    newMinY = Math.Max(newMinY, ctx.WorldBounds.MinY);
+                    newMaxY = Math.Min(newMaxY, ctx.WorldBounds.MaxY);
+                }
+                return ctx.MakeRectangle(newMinX, newMaxX, newMinY, newMaxY);
             }
         }
 
@@ -157,10 +154,9 @@ namespace Lucene.Net.Spatial.Prefix
         private sealed class _VisitorTemplate_121 : VisitorTemplate
         {
             private readonly WithinPrefixTreeFilter outerInstance;
-            private FixedBitSet inside;
 
+            private FixedBitSet inside;
             private FixedBitSet outside;
-
             private SpatialRelation visitRelation;
 
             public _VisitorTemplate_121(WithinPrefixTreeFilter outerInstance, AtomicReaderContext context, 
@@ -188,7 +184,6 @@ namespace Lucene.Net.Spatial.Prefix
                 return cell.GetSubCells(outerInstance.bufferedQueryShape).GetEnumerator();
             }
 
-            /// <exception cref="System.IO.IOException"></exception>
             protected internal override bool Visit(Cell cell)
             {
                 //cell.relate is based on the bufferedQueryShape; we need to examine what
@@ -236,7 +231,7 @@ namespace Lucene.Net.Spatial.Prefix
             /// Returns true if the provided cell, and all its sub-cells down to
             /// detailLevel all intersect the queryShape.
             /// </remarks>
-            private bool AllCellsIntersectQuery(Cell cell, SpatialRelation relate)
+            private bool AllCellsIntersectQuery(Cell cell, SpatialRelation relate/*cell to query*/)
             {
                 if (relate == SpatialRelation.NULL_VALUE)
                 {

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Query/SpatialArgs.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Query/SpatialArgs.cs b/src/Lucene.Net.Spatial/Query/SpatialArgs.cs
index 0ac68d2..83acc37 100644
--- a/src/Lucene.Net.Spatial/Query/SpatialArgs.cs
+++ b/src/Lucene.Net.Spatial/Query/SpatialArgs.cs
@@ -25,7 +25,10 @@ namespace Lucene.Net.Spatial.Queries
     {
         public static readonly double DEFAULT_DISTERRPCT = 0.025d;
 
-        public SpatialOperation Operation { get; set; }
+        private SpatialOperation operation;
+        private IShape shape;
+        private double? distErrPct;
+        private double? distErr;
 
         public SpatialArgs(SpatialOperation operation, IShape shape)
         {
@@ -67,13 +70,13 @@ namespace Lucene.Net.Spatial.Queries
 
         /// <summary>
         /// Gets the error distance that specifies how precise the query shape is. This
-        /// looks at {@link #getDistErr()}, {@link #getDistErrPct()}, and {@code
-        /// defaultDistErrPct}.
+        /// looks at <see cref="DistErr"/>, <see cref="DistErrPct"/>, and 
+        /// <paramref name="defaultDistErrPct"/>.
         /// </summary>
         /// <param name="ctx"></param>
         /// <param name="defaultDistErrPct">0 to 0.5</param>
         /// <returns>>= 0</returns>
-        public double ResolveDistErr(SpatialContext ctx, double defaultDistErrPct)
+        public virtual double ResolveDistErr(SpatialContext ctx, double defaultDistErrPct)
         {
             if (DistErr != null)
                 return DistErr.Value;
@@ -86,7 +89,7 @@ namespace Lucene.Net.Spatial.Queries
         /// </summary>
         public void Validate()
         {
-            if (Operation.IsTargetNeedsArea() && !Shape.HasArea)
+            if (Operation.IsTargetNeedsArea && !Shape.HasArea)
             {
                 throw new ArgumentException(Operation + " only supports geometry with area");
             }
@@ -97,7 +100,7 @@ namespace Lucene.Net.Spatial.Queries
             }
         }
 
-        public override String ToString()
+        public override string ToString()
         {
             return SpatialArgsParser.WriteSpatialArgs(this);
         }
@@ -106,7 +109,17 @@ namespace Lucene.Net.Spatial.Queries
         // Getters & Setters
         //------------------------------------------------
 
-        public IShape Shape { get; set; }
+        public virtual SpatialOperation Operation
+        {
+            get { return operation; }
+            set { operation = value; }
+        }
+
+        public virtual IShape Shape
+        {
+            get { return shape; }
+            set { shape = value; }
+        }
 
         /// <summary>
         /// A measure of acceptable error of the shape as a fraction. This effectively
@@ -115,22 +128,27 @@ namespace Lucene.Net.Spatial.Queries
         /// The default is {@link #DEFAULT_DIST_PRECISION}
         /// </summary>
         /// <returns>0 to 0.5</returns>
-        public double? DistErrPct
+        public virtual double? DistErrPct
         {
             get { return distErrPct; }
             set
             {
                 if (value != null)
-                    distErrPct = value.Value;
+                {
+                    distErrPct = value;
+                }
             }
         }
-        private double? distErrPct;
 
         /// <summary>
         /// The acceptable error of the shape.  This effectively inflates the
         /// size of the shape but should not shrink it.
         /// </summary>
         /// <returns>>= 0</returns>
-        public double? DistErr { get; set; }
+        public virtual double? DistErr
+        {
+            get { return distErr; }
+            set { distErr = value; }
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/lucenenet/blob/df3f64d7/src/Lucene.Net.Spatial/Query/SpatialArgsParser.cs
----------------------------------------------------------------------
diff --git a/src/Lucene.Net.Spatial/Query/SpatialArgsParser.cs b/src/Lucene.Net.Spatial/Query/SpatialArgsParser.cs
index 8ffad19..e71c9b5 100644
--- a/src/Lucene.Net.Spatial/Query/SpatialArgsParser.cs
+++ b/src/Lucene.Net.Spatial/Query/SpatialArgsParser.cs
@@ -20,27 +20,44 @@ using System.Collections.Generic;
 using System.Text;
 using Lucene.Net.Support;
 using Spatial4n.Core.Context;
+using Spatial4n.Core.Exceptions;
+using System.Linq;
+using Spatial4n.Core.Shapes;
 
 namespace Lucene.Net.Spatial.Queries
 {
+    /// <summary>
+    /// Parses a string that usually looks like "OPERATION(SHAPE)" into a {@link SpatialArgs}
+    /// object. The set of operations supported are defined in {@link SpatialOperation}, such
+    /// as "Intersects" being a common one. The shape portion is defined by WKT {@link com.spatial4j.core.io.WktShapeParser},
+    /// but it can be overridden/customized via {@link #parseShape(String, com.spatial4j.core.context.SpatialContext)}.
+    /// There are some optional name-value pair parameters that follow the closing parenthesis.  Example:
+    /// <code>
+    ///   Intersects(ENVELOPE(-10,-8,22,20)) distErrPct=0.025
+    /// </code>
+    /// <para/>
+    /// In the future it would be good to support something at least semi-standardized like a
+    /// variant of <a href="http://docs.geoserver.org/latest/en/user/filter/ecql_reference.html#spatial-predicate">
+    ///   [E]CQL</a>.
+    ///   
+    /// @lucene.experimental
+    /// </summary>
     public class SpatialArgsParser
     {
-        public const String DIST_ERR_PCT = "distErrPct";
-        public const String DIST_ERR = "distErr";
+        public const string DIST_ERR_PCT = "distErrPct";
+        public const string DIST_ERR = "distErr";
 
         /// <summary>
         /// Writes a close approximation to the parsed input format.
         /// </summary>
-        /// <param name="args"></param>
-        /// <returns></returns>
-        public static String WriteSpatialArgs(SpatialArgs args)
+        public static string WriteSpatialArgs(SpatialArgs args)
         {
             var str = new StringBuilder();
             str.Append(args.Operation.Name);
             str.Append('(');
             str.Append(args.Shape);
             if (args.DistErrPct != null)
-                str.Append(" distErrPct=").Append(String.Format("{0:0.00}%", args.DistErrPct * 100d));
+                str.Append(" distErrPct=").Append(string.Format("{0:0.00}%", args.DistErrPct * 100d));
             if (args.DistErr != null)
                 str.Append(" distErr=").Append(args.DistErr);
             str.Append(')');
@@ -50,45 +67,43 @@ namespace Lucene.Net.Spatial.Queries
         /// <summary>
         /// Parses a string such as "Intersects(-10,20,-8,22) distErrPct=0.025".
         /// </summary>
-        /// <param name="v"></param>
-        /// <param name="ctx"></param>
-        /// <returns></returns>
-        public SpatialArgs Parse(String v, SpatialContext ctx)
+        /// <param name="v">The string to parse. Mandatory.</param>
+        /// <param name="ctx">The spatial context. Mandatory.</param>
+        /// <returns>Not null.</returns>
+        /// <exception cref="ArgumentException">if the parameters don't make sense or an add-on parameter is unknown</exception>
+        /// <exception cref="ParseException">If there is a problem parsing the string</exception>
+        /// <exception cref="InvalidShapeException">When the coordinates are invalid for the shape</exception>
+        public SpatialArgs Parse(string v, SpatialContext ctx)
         {
             int idx = v.IndexOf('(');
             int edx = v.LastIndexOf(')');
 
             if (idx < 0 || idx > edx)
             {
-                throw new ArgumentException("missing parens: " + v);
+                throw new ParseException("missing parens: " + v, -1);
             }
 
-            SpatialOperation op = SpatialOperation.Get(v.Substring(0, idx).Trim());
+            SpatialOperation op = SpatialOperation.Get(v.Substring(0, idx - 0).Trim());
 
             //Substring in .NET is (startPosn, length), But in Java it's (startPosn, endPosn)
             //see http://docs.oracle.com/javase/1.4.2/docs/api/java/lang/String.html#substring(int, int)
-            String body = v.Substring(idx + 1, edx - (idx + 1)).Trim();
+            string body = v.Substring(idx + 1, edx - (idx + 1)).Trim();
             if (body.Length < 1)
             {
-                throw new ArgumentException("missing body : " + v);
+                throw new ParseException("missing body : " + v, idx + 1);
             }
 
-            var shape = ctx.ReadShape(body);
-            var args = new SpatialArgs(op, shape);
+            var shape = ParseShape(body, ctx);
+            var args = NewSpatialArgs(op, shape);
 
             if (v.Length > (edx + 1))
             {
                 body = v.Substring(edx + 1).Trim();
                 if (body.Length > 0)
                 {
-                    Dictionary<String, String> aa = ParseMap(body);
-                    args.DistErrPct = ReadDouble(aa[DIST_ERR_PCT]);
-                    aa.Remove(DIST_ERR_PCT);
-
-                    args.DistErr = ReadDouble(aa[DIST_ERR]);
-                    aa.Remove(DIST_ERR);
-
-                    if (aa.Count != 0)
+                    IDictionary<string, string> aa = ParseMap(body);
+                    ReadNameValuePairs(args, aa);
+                    if (!aa.Any())
                     {
                         throw new ArgumentException("unused parameters: " + aa);
                     }
@@ -98,13 +113,35 @@ namespace Lucene.Net.Spatial.Queries
             return args;
         }
 
-        protected static double? ReadDouble(String v)
+        protected virtual SpatialArgs NewSpatialArgs(SpatialOperation op, IShape shape)
+        {
+            return new SpatialArgs(op, shape);
+        }
+
+        protected virtual void ReadNameValuePairs(SpatialArgs args, IDictionary<string, string> nameValPairs)
+        {
+            string distErrPctStr, distErrStr;
+            nameValPairs.TryGetValue(DIST_ERR_PCT, out distErrPctStr);
+            nameValPairs.TryGetValue(DIST_ERR, out distErrStr);
+            args.DistErrPct = ReadDouble(distErrPctStr);
+            nameValPairs.Remove(DIST_ERR_PCT);
+            args.DistErr = ReadDouble(distErrStr);
+            nameValPairs.Remove(DIST_ERR);
+        }
+
+        protected virtual IShape ParseShape(string str, SpatialContext ctx) 
+        {
+            //return ctx.readShape(str);//still in Spatial4n 0.4 but will be deleted
+            return ctx.ReadShapeFromWkt(str);
+        }
+
+        protected static double? ReadDouble(string v)
         {
             double val;
             return double.TryParse(v, out val) ? val : (double?)null;
         }
 
-        protected static bool ReadBool(String v, bool defaultValue)
+        protected static bool ReadBool(string v, bool defaultValue)
         {
             bool ret;
             return bool.TryParse(v, out ret) ? ret : defaultValue;
@@ -114,21 +151,19 @@ namespace Lucene.Net.Spatial.Queries
         /// Parses "a=b c=d f" (whitespace separated) into name-value pairs. If there
         /// is no '=' as in 'f' above then it's short for f=f.
         /// </summary>
-        /// <param name="body"></param>
-        /// <returns></returns>
-        protected static Dictionary<String, String> ParseMap(String body)
+        protected static IDictionary<string, string> ParseMap(string body)
         {
-            var map = new Dictionary<String, String>();
+            var map = new Dictionary<string, string>();
             StringTokenizer st = new StringTokenizer(body, " \n\t");
 
             while (st.HasMoreTokens())
             {
-                String a = st.NextToken();
+                string a = st.NextToken();
                 int idx = a.IndexOf('=');
                 if (idx > 0)
                 {
-                    String k = a.Substring(0, idx);
-                    String v = a.Substring(idx + 1);
+                    string k = a.Substring(0, idx - 0);
+                    string v = a.Substring(idx + 1);
                     map[k] = v;
                 }
                 else