Merge pull request #312 from Kavec/path-readability

Refactor for readability

CONTRIBUTORS

@@ -20,5 +20,6 @@ Jeremy Jay <jeremy@pbnjay.com>
 Pius Uzamere <pius+github@alum.mit.edu>
 Robert Daniel Kortschak <dan.kortschak@adelaide.edu.au>
 Timothy Armstrong <armstrong.timothy@gmail.com>
+Tyler Gibbons <tyler.gibbons@ideogr.am>
 Zihua Li <i@zihua.li>
 Stefan Koshiw <anonwasere@gmail.com>

graph/path/morphism_apply_functions.go

@@ -20,58 +20,81 @@ import (
 	"github.com/google/cayley/quad"
 )
 
+// join puts two iterators together by intersecting their result sets with an AND
+// Since we're using an and iterator, it's a good idea to put the smallest result
+// set first so that Next() produces fewer values to check Contains().
+func join(qs graph.QuadStore, itL, itR graph.Iterator) graph.Iterator {
+	and := iterator.NewAnd(qs)
+	and.AddSubIterator(itL)
+	and.AddSubIterator(itR)
+
+	return and
+}
+
+// isMorphism represents all nodes passed in-- if there are none, this function
+// acts as a passthrough for the previous iterator.
 func isMorphism(nodes ...string) morphism {
 	return morphism{
 		Name:     "is",
 		Reversal: func() morphism { return isMorphism(nodes...) },
-		Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator {
+		Apply: func(qs graph.QuadStore, in graph.Iterator) graph.Iterator {
-			var sub graph.Iterator
 			if len(nodes) == 0 {
-				sub = qs.NodesAllIterator()
+				// Acting as a passthrough here is equivalent to
-			} else {
+				// building a NodesAllIterator to Next() or Contains()
-				fixed := qs.FixedIterator()
+				// from here as in previous versions.
-				for _, n := range nodes {
+				return in
-					fixed.Add(qs.ValueOf(n))
-				}
-				sub = fixed
 			}
-			and := iterator.NewAnd(qs)
+
-			and.AddSubIterator(sub)
+			isNodes := qs.FixedIterator()
-			and.AddSubIterator(it)
+			for _, n := range nodes {
-			return and
+				isNodes.Add(qs.ValueOf(n))
+			}
+
+			// Anything with fixedIterators will usually have a much
+			// smaller result set, so join isNodes first here.
+			return join(qs, isNodes, in)
 		},
 	}
 }
 
+// hasMorphism is the set of nodes that is reachable via either a *Path, a
+// single node.(string) or a list of nodes.([]string).
 func hasMorphism(via interface{}, nodes ...string) morphism {
 	return morphism{
 		Name:     "has",
 		Reversal: func() morphism { return hasMorphism(via, nodes...) },
-		Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator {
+		Apply: func(qs graph.QuadStore, in graph.Iterator) graph.Iterator {
-			var sub graph.Iterator
+			viaIter := buildViaPath(qs, via).
-			if len(nodes) == 0 {
+				BuildIterator()
-				sub = qs.NodesAllIterator()
+			ends := func() graph.Iterator {
-			} else {
+				if len(nodes) == 0 {
+					return qs.NodesAllIterator()
+				}
+
 				fixed := qs.FixedIterator()
 				for _, n := range nodes {
 					fixed.Add(qs.ValueOf(n))
 				}
-				sub = fixed
+				return fixed
+			}()
+
+			trail := iterator.NewLinksTo(qs, viaIter, quad.Predicate)
+			dest := iterator.NewLinksTo(qs, ends, quad.Object)
+
+			// If we were given nodes, intersecting with them first will
+			// be extremely cheap-- otherwise, it will be the most expensive
+			// (requiring iteration over all nodes). We have enough info to
+			// make this optimization now since intersections are commutative
+			if len(nodes) == 0 { // Where dest involves an All iterator.
+				route := join(qs, trail, dest)
+				has := iterator.NewHasA(qs, route, quad.Subject)
+				return join(qs, in, has)
 			}
-			var viaPath *Path
+
-			if via != nil {
+			// This looks backwards. That's OK-- see the note above.
-				viaPath = buildViaPath(qs, via)
+			route := join(qs, dest, trail)
-			} else {
+			has := iterator.NewHasA(qs, route, quad.Subject)
-				viaPath = buildViaPath(qs)
+			return join(qs, has, in)
-			}
-			subAnd := iterator.NewAnd(qs)
-			subAnd.AddSubIterator(iterator.NewLinksTo(qs, sub, quad.Object))
-			subAnd.AddSubIterator(iterator.NewLinksTo(qs, viaPath.BuildIterator(), quad.Predicate))
-			hasa := iterator.NewHasA(qs, subAnd, quad.Subject)
-			and := iterator.NewAnd(qs)
-			and.AddSubIterator(it)
-			and.AddSubIterator(hasa)
-			return and
 		},
 	}
 }
@@ -90,6 +113,7 @@ func tagMorphism(tags ...string) morphism {
 	}
 }
 
+// outMorphism iterates forward one RDF triple or via an entire path.
 func outMorphism(via ...interface{}) morphism {
 	return morphism{
 		Name:     "out",
@@ -101,6 +125,7 @@ func outMorphism(via ...interface{}) morphism {
 	}
 }
 
+// inMorphism iterates backwards one RDF triple or via an entire path.
 func inMorphism(via ...interface{}) morphism {
 	return morphism{
 		Name:     "in",
@@ -112,43 +137,42 @@ func inMorphism(via ...interface{}) morphism {
 	}
 }
 
+// iteratorMorphism simply tacks the input iterator onto the chain.
 func iteratorMorphism(it graph.Iterator) morphism {
 	return morphism{
 		Name:     "iterator",
 		Reversal: func() morphism { return iteratorMorphism(it) },
 		Apply: func(qs graph.QuadStore, subIt graph.Iterator) graph.Iterator {
-			and := iterator.NewAnd(qs)
+			return join(qs, it, subIt)
-			and.AddSubIterator(it)
-			and.AddSubIterator(subIt)
-			return and
 		},
 	}
 }
 
+// andMorphism sticks a path onto the current iterator chain.
 func andMorphism(p *Path) morphism {
 	return morphism{
 		Name:     "and",
 		Reversal: func() morphism { return andMorphism(p) },
-		Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator {
+		Apply: func(qs graph.QuadStore, itL graph.Iterator) graph.Iterator {
-			subIt := p.BuildIteratorOn(qs)
+			itR := p.BuildIteratorOn(qs)
-			and := iterator.NewAnd(qs)
+
-			and.AddSubIterator(it)
+			return join(qs, itL, itR)
-			and.AddSubIterator(subIt)
-			return and
 		},
 	}
 }
 
+// orMorphism is the union, vice intersection, of a path and the current iterator.
 func orMorphism(p *Path) morphism {
 	return morphism{
 		Name:     "or",
 		Reversal: func() morphism { return orMorphism(p) },
-		Apply: func(qs graph.QuadStore, it graph.Iterator) graph.Iterator {
+		Apply: func(qs graph.QuadStore, itL graph.Iterator) graph.Iterator {
-			subIt := p.BuildIteratorOn(qs)
+			itR := p.BuildIteratorOn(qs)
-			and := iterator.NewOr()
+
-			and.AddSubIterator(it)
+			or := iterator.NewOr()
-			and.AddSubIterator(subIt)
+			or.AddSubIterator(itL)
-			return and
+			or.AddSubIterator(itR)
+			return or
 		},
 	}
 }
@@ -163,17 +187,17 @@ func followMorphism(p *Path) morphism {
 	}
 }
 
+// exceptMorphism removes all results on p.(*Path) from the current iterators.
 func exceptMorphism(p *Path) morphism {
 	return morphism{
 		Name:     "except",
 		Reversal: func() morphism { return exceptMorphism(p) },
 		Apply: func(qs graph.QuadStore, base graph.Iterator) graph.Iterator {
-			subIt := p.BuildIteratorOn(qs)
+			in := p.BuildIteratorOn(qs)
-			notIt := iterator.NewNot(subIt, qs.NodesAllIterator())
+			allNodes := qs.NodesAllIterator()
-			and := iterator.NewAnd(qs)
+			notIn := iterator.NewNot(in, allNodes)
-			and.AddSubIterator(base)
+
-			and.AddSubIterator(notIt)
+			return join(qs, base, notIn)
-			return and
 		},
 	}
 }
@@ -200,40 +224,44 @@ func saveReverseMorphism(via interface{}, tag string) morphism {
 	}
 }
 
-func buildSave(qs graph.QuadStore, via interface{}, tag string, it graph.Iterator, reverse bool) graph.Iterator {
+func buildSave(
-	all := qs.NodesAllIterator()
+	qs graph.QuadStore, via interface{},
-	all.Tagger().Add(tag)
+	tag string, from graph.Iterator, reverse bool,
-	node, allDir := quad.Subject, quad.Object
+) graph.Iterator {
-	var viaPath *Path
+
-	if via != nil {
+	allNodes := qs.NodesAllIterator()
-		viaPath = buildViaPath(qs, via)
+	allNodes.Tagger().Add(tag)
-	} else {
+
-		viaPath = buildViaPath(qs)
+	start, goal := quad.Subject, quad.Object
-	}
 	if reverse {
-		node, allDir = allDir, node
+		start, goal = goal, start
 	}
-	lto := iterator.NewLinksTo(qs, all, allDir)
+	viaIter := buildViaPath(qs, via).
-	subAnd := iterator.NewAnd(qs)
+		BuildIterator()
-	subAnd.AddSubIterator(iterator.NewLinksTo(qs, viaPath.BuildIterator(), quad.Predicate))
+
-	subAnd.AddSubIterator(lto)
+	dest := iterator.NewLinksTo(qs, allNodes, goal)
-	hasa := iterator.NewHasA(qs, subAnd, node)
+	trail := iterator.NewLinksTo(qs, viaIter, quad.Predicate)
-	and := iterator.NewAnd(qs)
+
-	and.AddSubIterator(hasa)
+	route := join(qs, trail, dest)
-	and.AddSubIterator(it)
+	save := iterator.NewHasA(qs, route, start)
-	return and
+
+	return join(qs, from, save)
 }
 
-func inOutIterator(viaPath *Path, it graph.Iterator, reverse bool) graph.Iterator {
+func inOutIterator(viaPath *Path, from graph.Iterator, inIterator bool) graph.Iterator {
-	in, out := quad.Subject, quad.Object
+	start, goal := quad.Subject, quad.Object
-	if reverse {
+	if inIterator {
-		in, out = out, in
+		start, goal = goal, start
 	}
-	lto := iterator.NewLinksTo(viaPath.qs, it, in)
+
-	and := iterator.NewAnd(viaPath.qs)
+	viaIter := viaPath.BuildIterator()
-	and.AddSubIterator(iterator.NewLinksTo(viaPath.qs, viaPath.BuildIterator(), quad.Predicate))
+
-	and.AddSubIterator(lto)
+	source := iterator.NewLinksTo(viaPath.qs, from, start)
-	return iterator.NewHasA(viaPath.qs, and, out)
+	trail := iterator.NewLinksTo(viaPath.qs, viaIter, quad.Predicate)
+
+	route := join(viaPath.qs, source, trail)
+
+	return iterator.NewHasA(viaPath.qs, route, goal)
 }
 
 func buildViaPath(qs graph.QuadStore, via ...interface{}) *Path {

graph/path/path.go

@@ -140,11 +140,15 @@ func (p *Path) Except(path *Path) *Path {
 	return p
 }
 
+// Follow allows you to stitch two paths together. The resulting path will start
+// from where the first path left off and continue iterating down the path given.
 func (p *Path) Follow(path *Path) *Path {
 	p.stack = append(p.stack, followMorphism(path))
 	return p
 }
 
+// FollowReverse is the same as follow, except it will iterate backwards up the
+// path given as argument.
 func (p *Path) FollowReverse(path *Path) *Path {
 	p.stack = append(p.stack, followMorphism(path.Reverse()))
 	return p
@@ -163,7 +167,7 @@ func (p *Path) Save(via interface{}, tag string) *Path {
 }
 
 // SaveReverse is the same as Save, only in the reverse direction
-// (the subject of the linkage should be tagged, instead of the object)
+// (the subject of the linkage should be tagged, instead of the object).
 func (p *Path) SaveReverse(via interface{}, tag string) *Path {
 	p.stack = append(p.stack, saveReverseMorphism(via, tag))
 	return p