[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-177313470
  
Sorry on both counts. `SimpleMap*` is leakage from JENA-1084. My Git-fu 
failed. I thought I had kept that work separate. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[afs]

Github user afs commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-177312482
  
Merged after fixing up:

1/ I used "git pull --no-commit" and got some compile errors (imports of 
`TriOperator.Operator3` etc) in files `SimpleMap*`. These classes are unused so 
I deleted them before commiting.

2/ There were no licnese headers on `TConsumer3` etc. Please run `mvn clean 
test -Pdev` before submitting a pull request. This will run the license checks 
(also know as the RAT). Failing to pass RAT causes the maven build to fail.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/jena/pull/120


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-172877393
  
I'm happy to do whatever will get this merged and move on. I chose the 
prefix names and not the numerical suffix names because that is what Java does 
in (e.g.) `java.util.function.BiFunction` and I thought it was more important 
to be in line with Java itself than any library. I'll add a commit with the 
first proposal, although the use of a `T-` prefix seems really opaque. Without 
the context of this discussion, it's not all obvious to me why `T-` would mean 
"has the same type for all arguments".

As for `final`, I find `final` useful as an aid, not to the compiler, but 
to humans, but I'm happy to write against any style standard at all, as long as 
there is one. If "no `final` on interface params" is a rule, _please_ add it to 
[the only documentation I know 
of](https://jena.apache.org/getting_involved/reviewing_contributions.html#code-style)
 for such things.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ocymum]

Github user ocymum commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-172378843
  
@afs, I've put the new types in their own package and renamed away from 
using `Quad` as the prefix. Let me know what you think. I'd very much like to 
use this code to address JENA-1084 as well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[afs]

Github user afs commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-172391107
  
This is now quite inconsistent.  There too many names and both locations 
and naming are mixed up.

Let's simplify for jena-base.  What you call things in the "mem" package is 
your choice.

In tuple(t) and function(f) we now have: 

TetraConsumer (f), TetraFunction (f), Consumer4 (t), TetraOperator(t), 
TriConsumer(t), TriFunction(t), TriFunction.TriOperator(t). 
TriConsumer.Consumer3(t)

The common naming from other systems like fj (Functional Java), scala is 
numbering "F" and "Function4" etc - it's also aligned to the naming of Tuple 
implementations. Let's use that style and so we do not have concept clashes 
with RDF. Java has got itself into a bit of a hole on naming and from the 
consequences of erasure - no need to be restricted by that.

**Proposal 1:**

In package tuple:
```
@FunctionalInterface
interface TFunction4 { void apply(X x1, X x2, X x3, X x4) ; }
```
and 
```
@FunctionalInterface
interface TConsumer4 { void accept(X x1, X x2, X x3, X x4) ; }
```
same for `TFunction3` and `TConsumer3` and only these.

It's not a function on a tuple so not `TupleFunctionN`.  `T` keeps away 
from any future clash with `Function4`, `Consumer4`. We can put discussion of 
those elsewhere - not needed for this PR.

**Proposal 2:**

Have `function/Function4`, `function/Consumer4`.

These are the only ones. No Tuple-related all same X forms.
If package core/mem wants alias/specializations (and it does not really 
save very much) it can have them locally, maybe even short names like `F4` 
for `Function4`.


I much prefer proposal 1 and want leave "function" for a different 
discussion.

**Other**
If you want to add lots of `final` in core/mem, so be it. It's almost 
clutter and the compiler does calculate effectively final nowadays.

In particular, no final on interface parameters. The word is ignored, it 
does not take part in signature matching and it is not enforced in 
implementations.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on a diff in the pull request:

https://github.com/apache/jena/pull/120#discussion_r49275522
  
--- Diff: 
jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/QuadConsumer.java ---
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.jena.atlas.lib.tuple;
+
+import java.util.function.Consumer;
+
+/**
+ * Represents an operation that accepts four input arguments and returns 
no result. This is a four-arity specialization
+ * of {@link Consumer}. Unlike most other functional interfaces, {@code 
QuadConsumer} is expected to operate via
+ * side-effects.
+ * 
+ * This is a functional interface whose functional method is {@link 
#accept}.
+ *
+ * @param  the type of the first argument to the operation
+ * @param  the type of the second argument to the operation
+ * @param  the type of the third argument to the operation
+ * @param  the type of the fourth argument to the operation
+ * @see Consumer
+ */
+@FunctionalInterface
+public interface QuadConsumer {
+
+void accept(final W w, final X x, final Y y, final Z z);
+
+/**
+ * A specialization of {@link QuadConsumer} in which all arguments are 
of the same type.
+ *
--- End diff --

Where would you suggest that these new types go? (Perhaps another package 
in `jena-base`?) If they are not in this package, does that not vitiate your 
second point (about top-level vs. enclosed)? For a name, how about 
`TetraFunction` and `TetraOperator`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[afs]

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/120#discussion_r49250282
  
--- Diff: 
jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/QuadConsumer.java ---
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.jena.atlas.lib.tuple;
+
+import java.util.function.Consumer;
+
+/**
+ * Represents an operation that accepts four input arguments and returns 
no result. This is a four-arity specialization
+ * of {@link Consumer}. Unlike most other functional interfaces, {@code 
QuadConsumer} is expected to operate via
+ * side-effects.
+ * 
+ * This is a functional interface whose functional method is {@link 
#accept}.
+ *
+ * @param  the type of the first argument to the operation
+ * @param  the type of the second argument to the operation
+ * @param  the type of the third argument to the operation
+ * @param  the type of the fourth argument to the operation
+ * @see Consumer
+ */
+@FunctionalInterface
+public interface QuadConsumer {
+
+void accept(final W w, final X x, final Y y, final Z z);
+
+/**
+ * A specialization of {@link QuadConsumer} in which all arguments are 
of the same type.
+ *
--- End diff --

The Tuple package is about tuples of the same type.  Putting in 
QuadConsumer etc seems out of keeping with that.

`Consumer4` at the top level is more in keeping with the package which is 
about all the same ``.

In Jena `Quad` has overtones of a key concept which is different.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[afs]

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/120#discussion_r49172355
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/core/mem/OrderedTupleTable.java 
---
@@ -0,0 +1,140 @@
+/*
--- End diff --

The license header is differently formatted and in fact the original 
headers on some files have been reformatted.

Please use the same, standard header and layout.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[afs]

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/120#discussion_r49173681
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/core/mem/PMapQuadTable.java ---
@@ -37,13 +39,29 @@
  * use.
  *
  */
-public abstract class PMapQuadTable extends PMapTupleTableimplements QuadTable {
+public class PMapQuadTable extends PMapTupleTableimplements QuadTable {
+
+
+private static class QuadTupleMap extends TupleMap {
+
+private QuadTupleMap(final String mappedOrder) {
+super(mappedOrder, compileMapping("GSPO", mappedOrder));
+}
+}
+
--- End diff --

I don't see the point of this class (ditto`TripleTupleMap`).

`QuadTupleMap` is passed to the constructor `PMapTupleTable` as a 
`TupleMap`.  A static function will do the same for the `PMapTupleTable` 
constructor.
```
private static Tuplemap makeTupleMap(String mappedOrder)
{ return TupleMap.create("GSPO", mappedOrder) ; }
```
and 
```
 public PMapQuadTable(final String order) {
  this(order, makeTupleMap(order));
  }
```
More of a style point but I would make the code general and not hard-code 
GSPO into the index code.  Instead pass in a a pair when the index is made.  
with calls like """make index("GSPO", "SPOG")""".  Then changes to idnexing are 
all in one place.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[afs]

Github user afs commented on a diff in the pull request:

https://github.com/apache/jena/pull/120#discussion_r49173865
  
--- Diff: 
jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/TupleMap.java ---
@@ -46,7 +46,6 @@
  * {@code 0<-1, 1<-2, 2<-0} 
  * and the "putTransform" is where to place the items: {@code 0->2, 1->0, 
2->1}.
  */
-final
 public class TupleMap {
 /*
--- End diff --

Changes to `TupleMap` aren't needed.
See comments about QuadTupleMap.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-170022965
  
I've changed `TupleMap` back to `final`, with concomitant changes to the 
subclasses of `PMapTupleTable`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on a diff in the pull request:

https://github.com/apache/jena/pull/120#discussion_r49192464
  
--- Diff: 
jena-arq/src/main/java/org/apache/jena/sparql/core/mem/PMapQuadTable.java ---
@@ -37,13 +39,29 @@
  * use.
  *
  */
-public abstract class PMapQuadTable extends PMapTupleTableimplements QuadTable {
+public class PMapQuadTable extends PMapTupleTableimplements QuadTable {
+
+
+private static class QuadTupleMap extends TupleMap {
+
+private QuadTupleMap(final String mappedOrder) {
+super(mappedOrder, compileMapping("GSPO", mappedOrder));
+}
+}
+
--- End diff --

It seems significantly clearer to me this way (the `static` methods just 
delegate to constructors, and it's clear from the outside that they must) but I 
have no problem doing it the other way. I'll add a commit "resetting" 
`TupleMap` and using `static`s later today.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-170033484
  
Reformatted license headers.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-169384106
  
Third time is the charm!

I think this time I have a good factoring that begins to separate the 
notion of ordering from the kind of data structure which stores the nodes. This 
will be useful for JENA-1084, which I am also working on. I wish this could 
look a little cleaner, but I cannot separate the state for ordering and the 
state for node storage in the nicest way because of single-inheritance, or at 
least I can't see how to do that. (Suggestions welcome!)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

Github user ajs6f commented on the pull request:

https://github.com/apache/jena/pull/120#issuecomment-169393952
  
Oops! Adding a last commit to keep all the ordering machinery more 
centralized.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena pull request: JENA-1083: Factoring tuple ordering into TupleM...

[ajs6f]

GitHub user ajs6f opened a pull request:

https://github.com/apache/jena/pull/120

JENA-1083: Factoring tuple ordering into TupleMap



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/ajs6f/jena JENA-1083

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jena/pull/120.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #120


commit bcdb5aa2c304af0d71cb37fab80edbf80568a70b
Author: ajs6f 
Date:   2016-01-06T16:15:35Z

Factoring tuple ordering into TupleMap




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---