You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by ajs6f <gi...@git.apache.org> on 2015/12/10 21:22:41 UTC
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
GitHub user ajs6f opened a pull request:
https://github.com/apache/jena/pull/106
JENA-1083: Refactoring in TupleTable and subtypes for clarity and concision
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ajs6f/jena TupleTableCleanup
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/106.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 #106
----
commit 7769970b0124b3a054a20d315e44dce77c33e030
Author: ajs6f <aj...@virginia.edu>
Date: 2015-12-10T20:07:18Z
Refactoring in TupleTable and subtypes for clarity and concision
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/106#discussion_r47336271
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/mem/QuadTableForm.java ---
@@ -124,31 +96,21 @@ public PMapQuadTable get() {
return new PMapQuadTable(name()) {
@Override
- protected Quad quad(final Node s, final Node p, final Node o, final Node g) {
- return Quad.create(g, s, p, o);
- }
+ public Quad tuple(final Quad q) {
+ return create(q.getSubject(), q.getPredicate(), q.getObject(), q.getGraph());
--- End diff --
It'll be clear when the white space is standardised but am I right in seeing:
* Using Quad and putting strange things in the slots which is very confusing. `getGraph` returns the `getSubject`.
* There seems to be some creating objects just to take them apart again. The strength of the design is that quads/triples are not stored, instead nodes are placed in indexes. To reflect that, operations based on g/s/p/o in some order are clearer to me at least as well as reducing object churn.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/106#discussion_r47353822
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/mem/QuadTableForm.java ---
@@ -124,31 +96,21 @@ public PMapQuadTable get() {
return new PMapQuadTable(name()) {
@Override
- protected Quad quad(final Node s, final Node p, final Node o, final Node g) {
- return Quad.create(g, s, p, o);
- }
+ public Quad tuple(final Quad q) {
+ return create(q.getSubject(), q.getPredicate(), q.getObject(), q.getGraph());
--- End diff --
These criticisms are sadly accurate. I seem to have improved in one way by creating other defects. I'm going to close this PR and see if I can solve the problems I identified (repeated code, unclear responsibility) in a better, tighter way, perhaps involving `ColumnMap`.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/106#issuecomment-163886614
The whitespace changes make it tricky to follow the real changes so I have converted all the file in the package in master from tabs to spaces to give a common reference point.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/106#discussion_r47354583
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/mem/QuadTableForm.java ---
@@ -124,31 +96,21 @@ public PMapQuadTable get() {
return new PMapQuadTable(name()) {
@Override
- protected Quad quad(final Node s, final Node p, final Node o, final Node g) {
- return Quad.create(g, s, p, o);
- }
+ public Quad tuple(final Quad q) {
+ return create(q.getSubject(), q.getPredicate(), q.getObject(), q.getGraph());
--- End diff --
We have time. The release build for the vote has already been done.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the pull request:
https://github.com/apache/jena/pull/106#issuecomment-163739254
There are a lot of whitespace changes in this PR, for which I apologize. They are present because I have switched my IDE to 4-space indents to match the more commons usage in the Jena codebase. If this is too confusing and it would be preferably to go back to the form that I had been using, please say so and I will be happy to so do.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/106#discussion_r47353899
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/mem/QuadTableForm.java ---
@@ -124,31 +96,21 @@ public PMapQuadTable get() {
return new PMapQuadTable(name()) {
@Override
- protected Quad quad(final Node s, final Node p, final Node o, final Node g) {
- return Quad.create(g, s, p, o);
- }
+ public Quad tuple(final Quad q) {
+ return create(q.getSubject(), q.getPredicate(), q.getObject(), q.getGraph());
--- End diff --
To be clear, I will send that next PR in the next few days.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1083: Refactoring in TupleTable and subtyp...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f closed the pull request at:
https://github.com/apache/jena/pull/106
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---