You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2021/02/17 16:31:44 UTC

[GitHub] [jena] afs opened a new pull request #926: JENA-2049: Binding improvements

afs opened a new pull request #926:
URL: https://github.com/apache/jena/pull/926


   While this is a big PR, most of the files changed are because all uses of binding in Jena have been converted to the new code. The old way (`BindingMap`) still works.
   
   While making these changes, the process was to change some files to new style, then run the tests suite so that if behaviour changed, it was simpler to track down whether it was the translation of the usage code or the binding sub-system implementation.
   
   The old style is still there in `BindingFactory` as deprecated static methods to smooth migration. The original classes in case for code using the classes directly and not via the factory.
   
   The key new class is `BindingBuilder`.
   
   The main change in code is `BindingMap BindingFactory.create()` becomes `BindingBuilder Binding.builder()` and the effects of that.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] kinow commented on a change in pull request #926: JENA-2049: Binding improvements

Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #926:
URL: https://github.com/apache/jena/pull/926#discussion_r577926284



##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/engine/binding/Binding4.java
##########
@@ -0,0 +1,156 @@
+/*
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ *  See the NOTICE file distributed with this work for additional
+ *  information regarding copyright ownership.
+ */
+
+package org.apache.jena.sparql.engine.binding;
+
+import java.util.Iterator;
+import java.util.Objects;
+import java.util.function.BiConsumer;
+
+import org.apache.jena.graph.Node;
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.itr.Itr;
+
+/**
+ * A binding implementation that supports 3 binding pairs.

Review comment:
       s/3/4 ?

##########
File path: jena-extras/jena-querybuilder/src/test/java/org/apache/jena/arq/querybuilder/handlers/WhereHandlerTest.java
##########
@@ -710,27 +688,26 @@ public void testAddValueRow_pfx_collection() {
 		handler.addValueVar( query.getPrefixMapping(), v );
 		handler.addValueVar( query.getPrefixMapping(), x );
 		handler.addValueRow( query.getPrefixMapping(), Arrays.asList("<one>", "three"));
-		handler.addValueRow( query.getPrefixMapping(), Arrays.asList("<two>", "four" ));	
+		handler.addValueRow( query.getPrefixMapping(), Arrays.asList("<two>", "four" ));
 		handler.build();
-		
+
 		ElementData edat = new ElementData();
 		edat.add( v );
 		edat.add( x );
-	
-		BindingHashMap binding = new BindingHashMap();
-		binding.add( v, NodeFactory.createURI( "one" ));
-		binding.add( x, NodeFactory.createLiteral("three"));
-		edat.add( binding );
-		binding = new BindingHashMap();
-		binding.add( v, NodeFactory.createURI( "two" ));
-		binding.add( x, NodeFactory.createLiteral("four"));
-		edat.add( binding );
+        setupBindings(edat, x, v);

Review comment:
       Thought the space was wrong on this line, but looks like the rest of the file has tabs. Test file, no need to change it :+1: 

##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/engine/binding/BindingBuilder.java
##########
@@ -0,0 +1,486 @@
+/*
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ *  See the NOTICE file distributed with this work for additional
+ *  information regarding copyright ownership.
+ */
+
+package org.apache.jena.sparql.engine.binding;
+
+import static java.lang.String.format;
+
+import java.util.*;
+import java.util.function.BiConsumer;
+
+import org.apache.jena.atlas.lib.InternalErrorException;
+import org.apache.jena.graph.Node;
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.itr.Itr;
+import org.apache.jena.sparql.util.FmtUtils;
+
+/**
+ * Build Bindings.
+ * <p>
+ * {@linkplain Binding} are immutable and can not be changed once constructed. A
+ * {@code BindingBuilder} accumulates some var/node pairs and at he build step,

Review comment:
       "and at he build step" appears to be missing something?

##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/engine/binding/BindingBuilder.java
##########
@@ -0,0 +1,486 @@
+/*
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ *  See the NOTICE file distributed with this work for additional
+ *  information regarding copyright ownership.
+ */
+
+package org.apache.jena.sparql.engine.binding;
+
+import static java.lang.String.format;
+
+import java.util.*;
+import java.util.function.BiConsumer;
+
+import org.apache.jena.atlas.lib.InternalErrorException;
+import org.apache.jena.graph.Node;
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.itr.Itr;
+import org.apache.jena.sparql.util.FmtUtils;
+
+/**
+ * Build Bindings.
+ * <p>
+ * {@linkplain Binding} are immutable and can not be changed once constructed. A
+ * {@code BindingBuilder} accumulates some var/node pairs and at he build step,
+ * chooses the most efficient {@linkplain Binding} implementation.
+ *
+ * <pre>
+ *   Binding binding = Binding.builder()
+ *                        .add(var, value)
+ *                        ...
+ *                        .build();
+ * </pre>
+ *
+ * The {@link #build()} method can only be called once per binding constructed. A
+ * {@link BindingBuilder} can be reused but is not thread safe.
+ * <p>Example:
+ * <pre>
+ *   BindingBuilder builder = Binding.builder(parent);
+ *   for .... {
+ *
+ *      builder.reset()
+ *      for ( ... each variable ... ) {
+ *          builder.add(var, node)
+ *      }
+ *      Binding binding = builder.build();
+ *      ... accumulate binding ...
+ *   }
+ * </pre>
+ * <p>Example:
+ * <pre>
+ *   BindingBuilder builder = Binding.builder(parent);
+ *   for .... {
+ *      Binding binding =
+ *          builder.reset()
+ *             .add(var1, node1)
+ *             .add(var2, node2)
+ *             .add(var3, node3)
+ *             .build();
+ *      }
+ *      ... accumulate binding ...
+ *   }
+ * </pre>
+ *
+ * See also {@link BindingFactory}. When the number of var/node pairs is fixed and
+ * known, operations in {@link BindingFactory} directly create a binding without
+ * needing a builder.
+ */
+public class BindingBuilder
+{
+    // Do not implement .hashCode or .equals as value-based operations.
+    // Do not implement Binding or else it makes the coding accident of returning a
+    // BindingBuilder as a Binding result.

Review comment:
       :+1: 

##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/engine/binding/BindingBuilder.java
##########
@@ -0,0 +1,486 @@
+/*
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ *  See the NOTICE file distributed with this work for additional
+ *  information regarding copyright ownership.
+ */
+
+package org.apache.jena.sparql.engine.binding;
+
+import static java.lang.String.format;
+
+import java.util.*;
+import java.util.function.BiConsumer;
+
+import org.apache.jena.atlas.lib.InternalErrorException;
+import org.apache.jena.graph.Node;
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.itr.Itr;
+import org.apache.jena.sparql.util.FmtUtils;
+
+/**
+ * Build Bindings.
+ * <p>
+ * {@linkplain Binding} are immutable and can not be changed once constructed. A
+ * {@code BindingBuilder} accumulates some var/node pairs and at he build step,
+ * chooses the most efficient {@linkplain Binding} implementation.
+ *
+ * <pre>
+ *   Binding binding = Binding.builder()
+ *                        .add(var, value)
+ *                        ...
+ *                        .build();
+ * </pre>
+ *
+ * The {@link #build()} method can only be called once per binding constructed. A
+ * {@link BindingBuilder} can be reused but is not thread safe.
+ * <p>Example:
+ * <pre>
+ *   BindingBuilder builder = Binding.builder(parent);
+ *   for .... {
+ *
+ *      builder.reset()
+ *      for ( ... each variable ... ) {
+ *          builder.add(var, node)
+ *      }
+ *      Binding binding = builder.build();
+ *      ... accumulate binding ...
+ *   }
+ * </pre>
+ * <p>Example:
+ * <pre>
+ *   BindingBuilder builder = Binding.builder(parent);
+ *   for .... {
+ *      Binding binding =
+ *          builder.reset()
+ *             .add(var1, node1)
+ *             .add(var2, node2)
+ *             .add(var3, node3)
+ *             .build();
+ *      }
+ *      ... accumulate binding ...
+ *   }
+ * </pre>
+ *
+ * See also {@link BindingFactory}. When the number of var/node pairs is fixed and
+ * known, operations in {@link BindingFactory} directly create a binding without
+ * needing a builder.
+ */
+public class BindingBuilder
+{
+    // Do not implement .hashCode or .equals as value-based operations.
+    // Do not implement Binding or else it makes the coding accident of returning a
+    // BindingBuilder as a Binding result.
+
+    public static final Binding noParent = null;
+    /*package*/ static boolean CHECKING = true;
+    /*package*/ static final boolean UNIQUE_NAMES_CHECK = true;
+    // Additional checking for development. Set false in production.
+    /*package*/ static final boolean UNIQUE_NAMES_CHECK_PARENT = false;
+
+    private boolean haveBuilt = false;
+    protected final Binding parent;
+
+    // Optimise up to 4 slots
+    private Var  var1  = null;
+    private Node node1 = null;
+
+    private Var  var2  = null;
+    private Node node2 = null;
+
+    private Var  var3  = null;
+    private Node node3 = null;
+
+    private Var  var4  = null;
+    private Node node4 = null;
+
+    // More than 4
+    private Map<Var,Node> map = null;
+
+    private int countSlots() {
+        if ( map != null ) return map.size();
+        if ( var1 == null ) return 0;
+        if ( var2 == null ) return 1;
+        if ( var3 == null ) return 2;
+        if ( var4 == null ) return 3;
+        if ( map == null ) return 4;
+        throw new InternalErrorException("Inconsistent internal state");
+    }
+
+    public static BindingBuilder create() { return new BindingBuilder(); }
+    public static BindingBuilder create(Binding parent) { return new BindingBuilder(parent); }
+
+    /** Create via {@link #create()} */
+    /*package*/ BindingBuilder() { this(noParent); }
+
+    /** Create via {@link #create(Binding)} */
+    /*package*/ BindingBuilder(Binding parent) {
+        //super(parent);
+        this.parent = parent;
+    }
+
+    /**
+     * Accumulate (var,value) pairs.
+     * Allow binding in this level to be replaced (i.e. not in parent)
+     */
+    public BindingBuilder set(Var var, Node node) {
+        checkAdd(var, node, true);
+        set$(var, node);
+        return this;
+    }
+
+    // Accumulate (var,value) pairs.
+    public BindingBuilder add(Var var, Node node) {
+        checkAdd(var, node, false);
+        add$(var, node);
+        return this;
+    }
+
+    /** Add all the (var, value) pairs from another binding */
+    public BindingBuilder addAll(Binding other) {
+        Iterator<Var> vIter = other.vars();
+        for (; vIter.hasNext(); ) {
+            Var v = vIter.next();
+            Node n = other.get(v);
+            add(v,n);
+        }
+        return this;
+    }
+
+    private void add$(Var var, Node node)  {
+        if ( haveBuilt )
+            throw new IllegalStateException("Attempt to add a binding pair after build() without reset()");
+
+        if ( map != null ) {
+            map.put(var, node);
+            return;
+        }
+        if ( var1 == null ) {
+            var1 = var;
+            node1 = node;
+            return;
+        }
+        if ( var2 == null ) {
+            var2 = var;
+            node2 = node;
+            return;
+        }
+        if ( var3 == null ) {
+            var3 = var;
+            node3 = node;
+            return;
+        }
+        if ( var4 == null ) {
+            var4 = var;
+            node4 = node;
+            return;
+        }
+        // Spill from 4 to N. map is null.
+        map = new HashMap<>();
+        map.put(var1, node1);
+        map.put(var2, node2);
+        map.put(var3, node3);
+        map.put(var4, node4);
+        map.put(var, node);
+        var1 = null;
+        node1 = null;
+        var2 = null;
+        node2 = null;
+        var3 = null;
+        node3 = null;
+        var4 = null;
+        node4 = null;
+    }
+
+    private void set$(Var var, Node node)  {
+        if ( haveBuilt )
+            throw new IllegalStateException("Attempt to set a binding pair after build() without reset()");
+
+        if ( map != null ) {
+            map.put(var, node);
+            return;
+        }
+        if ( var1 == null || var.equals(var1) ) {
+            var1 = var;
+            node1 = node;
+            return;
+        }
+        if ( var2 == null || var.equals(var2) ) {
+            var2 = var;
+            node2 = node;
+            return;
+        }
+        if ( var3 == null || var.equals(var3) ) {
+            var3 = var;
+            node3 = node;
+            return;
+        }
+        if ( var4 == null || var.equals(var4) ) {
+            var4 = var;
+            node4 = node;
+            return;
+        }
+        // Spill from 4 to N.
+        // map is null.
+        map = new HashMap<>();
+        map.put(var1, node1);
+        map.put(var2, node2);
+        map.put(var3, node3);
+        map.put(var4, node4);
+        var1 = null;
+        node1 = null;
+        var2 = null;
+        node2 = null;
+        var3 = null;
+        node3 = null;
+        var4 = null;
+        node4 = null;
+        map.put(var, node);

Review comment:
       Took me a few seconds re-reading it to see that `$add` and `$set` are not identical :smile: need more :coffee: 

##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/graph/NodeTransformLib.java
##########
@@ -18,195 +18,228 @@
 
 package org.apache.jena.sparql.graph;
 
-import java.util.ArrayList ;
-import java.util.Iterator ;
-import java.util.List ;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
 import java.util.Objects;
 
-import org.apache.jena.atlas.iterator.Iter ;
-import org.apache.jena.graph.Node ;
-import org.apache.jena.graph.Triple ;
-import org.apache.jena.sparql.algebra.Op ;
-import org.apache.jena.sparql.algebra.Table ;
-import org.apache.jena.sparql.algebra.Transform ;
-import org.apache.jena.sparql.algebra.Transformer ;
-import org.apache.jena.sparql.algebra.table.TableData ;
+import org.apache.jena.atlas.iterator.Iter;
+import org.apache.jena.graph.Node;
+import org.apache.jena.graph.Triple;
+import org.apache.jena.sparql.algebra.Op;
+import org.apache.jena.sparql.algebra.Table;
+import org.apache.jena.sparql.algebra.Transform;
+import org.apache.jena.sparql.algebra.Transformer;
+import org.apache.jena.sparql.algebra.table.TableData;
 import org.apache.jena.sparql.core.*;
-import org.apache.jena.sparql.engine.binding.Binding ;
-import org.apache.jena.sparql.engine.binding.BindingFactory ;
-import org.apache.jena.sparql.engine.binding.BindingMap ;
-import org.apache.jena.sparql.expr.Expr ;
-import org.apache.jena.sparql.expr.ExprList ;
+import org.apache.jena.sparql.engine.binding.Binding;
+import org.apache.jena.sparql.engine.binding.BindingBuilder;
+import org.apache.jena.sparql.expr.Expr;
+import org.apache.jena.sparql.expr.ExprList;
 import org.apache.jena.sparql.expr.ExprTransform;
 
-public class NodeTransformLib
-{
-    /** Do a node{@literal ->}node conversion of an Op - return original BGP for "no change" */
+public class NodeTransformLib {
+    /**
+     * Do a node{@literal ->}node conversion of an Op - return original BGP for "no

Review comment:
       Ah, I've been using `{@code ->}` when I needed something like this in Javadocs. Learned something new reviewing this PR. Thanks!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs merged pull request #926: JENA-2049: Binding improvements

Posted by GitBox <gi...@apache.org>.
afs merged pull request #926:
URL: https://github.com/apache/jena/pull/926


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #926: JENA-2049: Binding improvements

Posted by GitBox <gi...@apache.org>.
afs commented on pull request #926:
URL: https://github.com/apache/jena/pull/926#issuecomment-781271720


   Comments responded to and fixed in the force-pushed updates. 
   
   Trying to make this a single commit.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on a change in pull request #926: JENA-2049: Binding improvements

Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #926:
URL: https://github.com/apache/jena/pull/926#discussion_r577995128



##########
File path: jena-extras/jena-querybuilder/src/test/java/org/apache/jena/arq/querybuilder/handlers/WhereHandlerTest.java
##########
@@ -710,27 +688,26 @@ public void testAddValueRow_pfx_collection() {
 		handler.addValueVar( query.getPrefixMapping(), v );
 		handler.addValueVar( query.getPrefixMapping(), x );
 		handler.addValueRow( query.getPrefixMapping(), Arrays.asList("<one>", "three"));
-		handler.addValueRow( query.getPrefixMapping(), Arrays.asList("<two>", "four" ));	
+		handler.addValueRow( query.getPrefixMapping(), Arrays.asList("<two>", "four" ));
 		handler.build();
-		
+
 		ElementData edat = new ElementData();
 		edat.add( v );
 		edat.add( x );
-	
-		BindingHashMap binding = new BindingHashMap();
-		binding.add( v, NodeFactory.createURI( "one" ));
-		binding.add( x, NodeFactory.createLiteral("three"));
-		edat.add( binding );
-		binding = new BindingHashMap();
-		binding.add( v, NodeFactory.createURI( "two" ));
-		binding.add( x, NodeFactory.createLiteral("four"));
-		edat.add( binding );
+        setupBindings(edat, x, v);

Review comment:
       Old code had tabs.
   
   This is why tabs are bad - display in GH is not as context sensitive as an IDE.

##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/engine/binding/BindingBuilder.java
##########
@@ -0,0 +1,486 @@
+/*
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ *  See the NOTICE file distributed with this work for additional
+ *  information regarding copyright ownership.
+ */
+
+package org.apache.jena.sparql.engine.binding;
+
+import static java.lang.String.format;
+
+import java.util.*;
+import java.util.function.BiConsumer;
+
+import org.apache.jena.atlas.lib.InternalErrorException;
+import org.apache.jena.graph.Node;
+import org.apache.jena.sparql.core.Var;
+import org.apache.jena.sparql.engine.binding.itr.Itr;
+import org.apache.jena.sparql.util.FmtUtils;
+
+/**
+ * Build Bindings.
+ * <p>
+ * {@linkplain Binding} are immutable and can not be changed once constructed. A
+ * {@code BindingBuilder} accumulates some var/node pairs and at he build step,
+ * chooses the most efficient {@linkplain Binding} implementation.
+ *
+ * <pre>
+ *   Binding binding = Binding.builder()
+ *                        .add(var, value)
+ *                        ...
+ *                        .build();
+ * </pre>
+ *
+ * The {@link #build()} method can only be called once per binding constructed. A
+ * {@link BindingBuilder} can be reused but is not thread safe.
+ * <p>Example:
+ * <pre>
+ *   BindingBuilder builder = Binding.builder(parent);
+ *   for .... {
+ *
+ *      builder.reset()
+ *      for ( ... each variable ... ) {
+ *          builder.add(var, node)
+ *      }
+ *      Binding binding = builder.build();
+ *      ... accumulate binding ...
+ *   }
+ * </pre>
+ * <p>Example:
+ * <pre>
+ *   BindingBuilder builder = Binding.builder(parent);
+ *   for .... {
+ *      Binding binding =
+ *          builder.reset()
+ *             .add(var1, node1)
+ *             .add(var2, node2)
+ *             .add(var3, node3)
+ *             .build();
+ *      }
+ *      ... accumulate binding ...
+ *   }
+ * </pre>
+ *
+ * See also {@link BindingFactory}. When the number of var/node pairs is fixed and
+ * known, operations in {@link BindingFactory} directly create a binding without
+ * needing a builder.
+ */
+public class BindingBuilder
+{
+    // Do not implement .hashCode or .equals as value-based operations.
+    // Do not implement Binding or else it makes the coding accident of returning a
+    // BindingBuilder as a Binding result.
+
+    public static final Binding noParent = null;
+    /*package*/ static boolean CHECKING = true;
+    /*package*/ static final boolean UNIQUE_NAMES_CHECK = true;
+    // Additional checking for development. Set false in production.
+    /*package*/ static final boolean UNIQUE_NAMES_CHECK_PARENT = false;
+
+    private boolean haveBuilt = false;
+    protected final Binding parent;
+
+    // Optimise up to 4 slots
+    private Var  var1  = null;
+    private Node node1 = null;
+
+    private Var  var2  = null;
+    private Node node2 = null;
+
+    private Var  var3  = null;
+    private Node node3 = null;
+
+    private Var  var4  = null;
+    private Node node4 = null;
+
+    // More than 4
+    private Map<Var,Node> map = null;
+
+    private int countSlots() {
+        if ( map != null ) return map.size();
+        if ( var1 == null ) return 0;
+        if ( var2 == null ) return 1;
+        if ( var3 == null ) return 2;
+        if ( var4 == null ) return 3;
+        if ( map == null ) return 4;
+        throw new InternalErrorException("Inconsistent internal state");
+    }
+
+    public static BindingBuilder create() { return new BindingBuilder(); }
+    public static BindingBuilder create(Binding parent) { return new BindingBuilder(parent); }
+
+    /** Create via {@link #create()} */
+    /*package*/ BindingBuilder() { this(noParent); }
+
+    /** Create via {@link #create(Binding)} */
+    /*package*/ BindingBuilder(Binding parent) {
+        //super(parent);
+        this.parent = parent;
+    }
+
+    /**
+     * Accumulate (var,value) pairs.
+     * Allow binding in this level to be replaced (i.e. not in parent)
+     */
+    public BindingBuilder set(Var var, Node node) {
+        checkAdd(var, node, true);
+        set$(var, node);
+        return this;
+    }
+
+    // Accumulate (var,value) pairs.
+    public BindingBuilder add(Var var, Node node) {
+        checkAdd(var, node, false);
+        add$(var, node);
+        return this;
+    }
+
+    /** Add all the (var, value) pairs from another binding */
+    public BindingBuilder addAll(Binding other) {
+        Iterator<Var> vIter = other.vars();
+        for (; vIter.hasNext(); ) {
+            Var v = vIter.next();
+            Node n = other.get(v);
+            add(v,n);
+        }
+        return this;
+    }
+
+    private void add$(Var var, Node node)  {
+        if ( haveBuilt )
+            throw new IllegalStateException("Attempt to add a binding pair after build() without reset()");
+
+        if ( map != null ) {
+            map.put(var, node);
+            return;
+        }
+        if ( var1 == null ) {
+            var1 = var;
+            node1 = node;
+            return;
+        }
+        if ( var2 == null ) {
+            var2 = var;
+            node2 = node;
+            return;
+        }
+        if ( var3 == null ) {
+            var3 = var;
+            node3 = node;
+            return;
+        }
+        if ( var4 == null ) {
+            var4 = var;
+            node4 = node;
+            return;
+        }
+        // Spill from 4 to N. map is null.
+        map = new HashMap<>();
+        map.put(var1, node1);
+        map.put(var2, node2);
+        map.put(var3, node3);
+        map.put(var4, node4);
+        map.put(var, node);
+        var1 = null;
+        node1 = null;
+        var2 = null;
+        node2 = null;
+        var3 = null;
+        node3 = null;
+        var4 = null;
+        node4 = null;
+    }
+
+    private void set$(Var var, Node node)  {
+        if ( haveBuilt )
+            throw new IllegalStateException("Attempt to set a binding pair after build() without reset()");
+
+        if ( map != null ) {
+            map.put(var, node);
+            return;
+        }
+        if ( var1 == null || var.equals(var1) ) {
+            var1 = var;
+            node1 = node;
+            return;
+        }
+        if ( var2 == null || var.equals(var2) ) {
+            var2 = var;
+            node2 = node;
+            return;
+        }
+        if ( var3 == null || var.equals(var3) ) {
+            var3 = var;
+            node3 = node;
+            return;
+        }
+        if ( var4 == null || var.equals(var4) ) {
+            var4 = var;
+            node4 = node;
+            return;
+        }
+        // Spill from 4 to N.
+        // map is null.
+        map = new HashMap<>();
+        map.put(var1, node1);
+        map.put(var2, node2);
+        map.put(var3, node3);
+        map.put(var4, node4);
+        var1 = null;
+        node1 = null;
+        var2 = null;
+        node2 = null;
+        var3 = null;
+        node3 = null;
+        var4 = null;
+        node4 = null;
+        map.put(var, node);

Review comment:
       ... or maybe less :coffee: 😄 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org