add Neo4j heuristics calculator#1647
Conversation
| Truthness inner = evaluateCondition(((NotCondition) condition).getCondition(), mapping); | ||
| return inner == null ? null : inner.invert(); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
instead of null shouldn't this throw an exception signalling that a case was not supported?
| * Returns {@code ρ(condition, mapping)}, or {@code null} when the condition cannot be evaluated | ||
| * and must be skipped by the aggregation. | ||
| */ | ||
| Truthness evaluateCondition(CypherCondition condition, Neo4jMapping mapping) { |
There was a problem hiding this comment.
have you considered applying a visitor pattern to CypherCondition AST ?
| Double da = asDouble(a); | ||
| Double db = asDouble(b); | ||
| if (da == null || db == null) { | ||
| return null; |
There was a problem hiding this comment.
is null a valid input for this method?
|
|
||
| private Truthness equalityTruthness(Object a, Object b) { | ||
| if (a == null || b == null) { | ||
| return null; |
There was a problem hiding this comment.
what if two references are null? what is the Neo4J semantics for this?
| */ | ||
| static Truthness stringEqualityTruthness(String a, String b) { | ||
| if (a == null || b == null) { | ||
| return null; |
There was a problem hiding this comment.
is a and b nullable? what is the semantics?
| } | ||
|
|
||
| static Truthness getStartsWith(String str, String prefix) { | ||
| if (str.startsWith(prefix)) { |
There was a problem hiding this comment.
add Objects.requireNonNull(...) whenever the arguments are intended not to be null.
| * {@code [0,1]}, where 0 means the query is satisfied. | ||
| */ | ||
| public double computeDistance(MatchOperation query, Neo4jGraph graph) { | ||
| return 1.0d - computeHeuristic(query, graph).getOfTrue(); |
There was a problem hiding this comment.
add a variable to store computeHeuristic(query, graph)
| * Count-based node availability: enough graph nodes to bind the pattern's nodes. Pure cardinality, | ||
| * no label/property check (those are conditions evaluated by H_where). | ||
| */ | ||
| Truthness computeHeuristicMatchNodes(int required, int available) { |
There was a problem hiding this comment.
can values be negative? Add checks with IllegalArgumentException to signal invalid inputs.
| return new Neo4jMapping(nodeBindings, edgeBindings); | ||
| } | ||
|
|
||
| Neo4jNode getNode(String variable) { |
There was a problem hiding this comment.
add Objects.requireNonNull(...) if needed
First of a few PRs splitting #1641, per review feedback.
H(Q,G): how close a graph is to satisfying a parsed Cypher MATCH query, as a Truthness<ofTrue, ofFalse>matched_elements), Neo4jConditionEvaluator (condition + operand evaluation), Neo4jMapping