Refactoring/cleanup/fixes.

internal/server/expression.go

@@ -12,6 +12,11 @@ const (
 	NOOP BinaryOp = iota
 	AND
 	OR
+	tokenAND    = "and"
+	tokenOR     = "or"
+	tokenNOT    = "not"
+	tokenLParen = "("
+	tokenRParen = ")"
 )
 
 // areaExpression is either an object or operator+children
@@ -24,169 +29,214 @@ type areaExpression struct {
 
 type children []*areaExpression
 
-func (e *areaExpression) String() string {
+func (e *areaExpression) String() (res string) {
 	if e.obj != nil {
-		return e.obj.String()
+		res = e.obj.String()
+	} else {
+		var chStrings []string
+		for _, c := range e.children {
+			chStrings = append(chStrings, c.String())
+		}
+		switch e.op {
+		case NOOP:
+			res = "empty operator"
+		case AND:
+			res = "(" + strings.Join(chStrings, " "+tokenAND+" ") + ")"
+		case OR:
+			res = "(" + strings.Join(chStrings, " "+tokenOR+" ") + ")"
+		default:
+			res = "unknown operator"
+		}
 	}
-	var chStrings []string
+	if e.negate {
-	for _, c := range e.children {
+		res = tokenNOT + " " + res
-		chStrings = append(chStrings, c.String())
 	}
-	switch e.op {
+	return
-	case NOOP:
-		return "empty operator"
-	case AND:
-		return "(" + strings.Join(chStrings, " AND ") + ")"
-	case OR:
-		return "(" + strings.Join(chStrings, " OR ") + ")"
-	}
-	return "unknown operator"
 }
 
 // Return boolean value modulo negate field of the expression.
-func (e *areaExpression) booleanize(val bool) bool {
+func (e *areaExpression) maybeNegate(val bool) bool {
 	if e.negate {
 		return !val
 	}
 	return val
 }
 
-func (e *areaExpression) Intersects(o geojson.Object) bool {
+// Methods for testing an areaExpression against the spatial object
+func (e *areaExpression) rawIntersects(o geojson.Object) bool {
 	if e.obj != nil {
-		return e.booleanize(e.obj.Intersects(o))
+		return e.obj.Intersects(o)
 	}
 	switch e.op {
 	case AND:
 		for _, c := range e.children {
 			if !c.Intersects(o) {
-				return e.booleanize(false)
+				return false
 			}
 		}
-		return e.booleanize(true)
+		return true
 	case OR:
 		for _, c := range e.children {
 			if c.Intersects(o) {
-				return e.booleanize(true)
+				return true
 			}
 		}
-		return e.booleanize(false)
+		return false
 	}
-	return e.booleanize(false)
+	return false
 }
 
-// object within an expression means anything of this expression contains object
+func (e *areaExpression) rawContains(o geojson.Object) bool {
-func (e *areaExpression) Contains(o geojson.Object) bool {
 	if e.obj != nil {
-		return e.booleanize(e.obj.Contains(o))
+		return e.obj.Contains(o)
 	}
 	switch e.op {
 	case AND:
 		for _, c:= range e.children {
 			if !c.Contains(o) {
-				return e.booleanize(false)
+				return false
 			}
 		}
-		return e.booleanize(true)
+		return true
 	case OR:
 		for _, c:= range e.children {
 			if c.Contains(o) {
-				return e.booleanize(true)
+				return true
 			}
 		}
-		return e.booleanize(false)
+		return false
 	}
-	return e.booleanize(false)
+	return false
 }
 
-func (e *areaExpression) Within(o geojson.Object) bool {
+func (e *areaExpression) rawWithin(o geojson.Object) bool {
 	if e.obj != nil {
-		return e.booleanize(e.obj.Within(o))
+		return e.obj.Within(o)
 	}
 	switch e.op {
 	case AND:
 		for _, c:= range e.children {
 			if !c.Within(o) {
-				return e.booleanize(false)
+				return false
 			}
 		}
-		return e.booleanize(true)
+		return true
 	case OR:
 		for _, c:= range e.children {
 			if c.Within(o) {
-				return e.booleanize(true)
+				return true
 			}
 		}
-		return e.booleanize(false)
+		return false
 	}
-	return e.booleanize(false)
+	return false
+}
+
+func (e *areaExpression) Intersects(o geojson.Object) bool {
+	return e.maybeNegate(e.rawIntersects(o))
+}
+
+func (e *areaExpression) Contains(o geojson.Object) bool {
+	return e.maybeNegate(e.rawContains(o))
+}
+
+func (e *areaExpression) Within(o geojson.Object) bool {
+	return e.maybeNegate(e.rawWithin(o))
+}
+
+// Methods for testing an areaExpression against another areaExpression
+func (e *areaExpression) rawIntersectsExpr(oe *areaExpression) bool {
+	if oe.negate {
+		e2 := &areaExpression{negate:!e.negate, obj:e.obj, op: e.op, children:e.children}
+		oe2 := &areaExpression{negate:false, obj:oe.obj, op:oe.op, children:oe.children}
+		return e2.rawIntersectsExpr(oe2)
+	}
+	if oe.obj != nil {
+		return e.Intersects(oe.obj)
+	}
+	switch oe.op {
+	case AND:
+		for _, c := range oe.children {
+			if !e.rawIntersectsExpr(c) {
+				return false
+			}
+		}
+		return true
+	case OR:
+		for _, c := range oe.children {
+			if e.rawIntersectsExpr(c) {
+				return true
+			}
+		}
+		return false
+	}
+	return false
+}
+
+func (e *areaExpression) rawWithinExpr(oe *areaExpression) bool {
+	if oe.negate {
+		e2 := &areaExpression{negate:!e.negate, obj:e.obj, op: e.op, children:e.children}
+		oe2 := &areaExpression{negate:false, obj:oe.obj, op:oe.op, children:oe.children}
+		return e2.rawWithinExpr(oe2)
+	}
+	if oe.obj != nil {
+		return e.Within(oe.obj)
+	}
+	switch oe.op {
+	case AND:
+		for _, c:= range oe.children {
+			if !e.rawWithinExpr(c) {
+				return false
+			}
+		}
+		return true
+	case OR:
+		for _, c:= range oe.children {
+			if e.rawWithinExpr(c) {
+				return true
+			}
+		}
+		return false
+	}
+	return false
+}
+
+func (e *areaExpression) rawContainsExpr(oe *areaExpression) bool {
+	if oe.negate {
+		e2 := &areaExpression{negate:!e.negate, obj:e.obj, op: e.op, children:e.children}
+		oe2 := &areaExpression{negate:false, obj:oe.obj, op:oe.op, children:oe.children}
+		return e2.rawContainsExpr(oe2)
+	}
+	if oe.obj != nil {
+		return e.Contains(oe.obj)
+	}
+	switch oe.op {
+	case AND:
+		for _, c:= range oe.children {
+			if !e.rawContainsExpr(c) {
+				return false
+			}
+		}
+		return true
+	case OR:
+		for _, c:= range oe.children {
+			if e.rawContainsExpr(c) {
+				return true
+			}
+		}
+		return false
+	}
+	return false
 }
 
 func (e *areaExpression) IntersectsExpr(oe *areaExpression) bool {
-	if oe.obj != nil {
+	return e.maybeNegate(e.rawIntersectsExpr(oe))
-		return oe.booleanize(e.Intersects(oe.obj))
-	}
-	switch oe.op {
-	case AND:
-		for _, c := range oe.children {
-			if !e.IntersectsExpr(c) {
-				return e.booleanize(false)
-			}
-		}
-		return e.booleanize(true)
-	case OR:
-		for _, c := range oe.children {
-			if e.IntersectsExpr(c) {
-				return e.booleanize(true)
-			}
-		}
-		return e.booleanize(false)
-	}
-	return e.booleanize(false)
-
 }
 
 func (e *areaExpression) WithinExpr(oe *areaExpression) bool {
-	if oe.obj != nil {
+	return e.maybeNegate(e.rawWithinExpr(oe))
-		return oe.booleanize(e.Within(oe.obj))
-	}
-	switch oe.op {
-	case AND:
-		for _, c:= range oe.children {
-			if !e.WithinExpr(c) {
-				return e.booleanize(false)
-			}
-		}
-		return e.booleanize(true)
-	case OR:
-		for _, c:= range oe.children {
-			if e.WithinExpr(c) {
-				return e.booleanize(true)
-			}
-		}
-		return e.booleanize(false)
-	}
-	return e.booleanize(false)
 }
 
 func (e *areaExpression) ContainsExpr(oe *areaExpression) bool {
-	if oe.obj != nil {
+	return e.maybeNegate(e.rawContainsExpr(oe))
-		return oe.booleanize(e.Contains(oe.obj))
-	}
-	switch oe.op {
-	case AND:
-		for _, c:= range oe.children {
-			if !e.ContainsExpr(c) {
-				return e.booleanize(false)
-			}
-		}
-		return e.booleanize(true)
-	case OR:
-		for _, c:= range oe.children {
-			if e.ContainsExpr(c) {
-				return e.booleanize(true)
-			}
-		}
-		return e.booleanize(false)
-	}
-	return e.booleanize(false)
 }

internal/server/token.go

@@ -720,7 +720,7 @@ loop:
 			break
 		}
 		switch strings.ToLower(wtok) {
-		case "(":
+		case tokenLParen:
 			newExpr := &areaExpression{negate: negate, op: NOOP}
 			negate = false
 			if ae != nil {
@@ -729,28 +729,28 @@ loop:
 			}
 			ae = newExpr
 			vsout = nvs
-		case ")":
+		case tokenRParen:
 			if negate {
-				err = errInvalidArgument("NOT")
+				err = errInvalidArgument(tokenNOT)
 				return
 			}
 			if parent, empty := ps.pop(); empty {
-				err = errInvalidArgument(")")
+				err = errInvalidArgument(tokenRParen)
 				return
 			} else {
 				ae = parent
 			}
 			vsout = nvs
-		case "not":
+		case tokenNOT:
 			negate = true
 			vsout = nvs
-		case "and":
+		case tokenAND:
 			if negate {
-				err = errInvalidArgument("NOT")
+				err = errInvalidArgument(tokenNOT)
 				return
 			}
 			if ae == nil {
-				err = errInvalidArgument("AND")
+				err = errInvalidArgument(tokenAND)
 				return
 			} else if ae.obj == nil {
 				switch ae.op {
@@ -773,13 +773,13 @@ loop:
 				ae = &areaExpression{op: AND, children: []*areaExpression{ae}}
 			}
 			vsout = nvs
-		case "or":
+		case tokenOR:
 			if negate {
-				err = errInvalidArgument("NOT")
+				err = errInvalidArgument(tokenNOT)
 				return
 			}
 			if ae == nil {
-				err = errInvalidArgument("OR")
+				err = errInvalidArgument(tokenOR)
 				return
 			} else if ae.obj == nil {
 				switch ae.op {
@@ -820,7 +820,7 @@ loop:
 			}
 		default:
 			if negate {
-				err = errInvalidArgument("NOT")
+				err = errInvalidArgument(tokenNOT)
 				return
 			}
 			break loop

tests/testcmd_test.go

@@ -162,5 +162,4 @@ func testcmd_expression_test(mc *mockServer) error {
 			"(", "OBJECT", poly, "AND", "NOT", "GET", "mykey", "line3", ")"}, {"1"},
 		{"TEST", "OBJECT", poly9, "WITHIN", "NOT", "GET", "mykey", "line3"}, {"1"},
 	})
-
 }