You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by afs <gi...@git.apache.org> on 2015/12/28 18:39:19 UTC

[GitHub] jena pull request: JENA-1107 : Improve Tuple.

GitHub user afs opened a pull request:

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

    JENA-1107 : Improve Tuple.

    Improvements:
    * Tuple as Interface
    * Implementation focused on being "Tuples", not only for TDB (it's historical origin).
    * Less space for small tuples.
    * Avoid array overheads for small tuples.
    * TupleMap uses more consistent naming than ColumnMap.
    * TupleMap can be used for index mapping, array mapping, not use Tuple mapping.

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

    $ git pull https://github.com/afs/jena master

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

    https://github.com/apache/jena/pull/115.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 #115
    
----
commit 571e97553c9d5ccb29818ed85afb70e4fe0318c2
Author: Andy Seaborne <an...@seaborne.org>
Date:   2015-12-28T16:27:01Z

    JENA-1107: Rework Tuple

commit c665f63004fee2d465f2cbcb48d2defd17e12858
Author: Andy Seaborne <an...@seaborne.org>
Date:   2015-12-28T16:27:48Z

    JENA-1107: Changes due to reworking Tuple.

commit 9687f8368960d6505d910313d752a68194dbea23
Author: Andy Seaborne <an...@seaborne.org>
Date:   2015-12-28T16:57:39Z

    Tuple.asArray

commit b5d593d2d132e23533937924891a6d642afe32f2
Author: Andy Seaborne <an...@seaborne.org>
Date:   2015-12-28T17:34:56Z

    Clean up whitespace inconsistencies

----


---
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-1107 : Improve Tuple.

Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:

    https://github.com/apache/jena/pull/115#discussion_r48498336
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/Tuple.java ---
    @@ -0,0 +1,73 @@
    +/*
    + * 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.List ;
    +import java.util.function.Consumer ;
    +import java.util.stream.Stream ;
    +
    +import org.apache.jena.atlas.lib.ArrayUtils ;
    +
    +/** A Tuple is the same class of item */
    +public interface Tuple<X> extends Iterable<X> {
    +    /** Get the i'th element, for i in the range 0 to len()-1 
    +     * @throws IndexOutOfBoundsException for i out of range 
    +     */
    +    public X get(int i) ;
    +
    +    /** length : elements are 0 to len()-1 */ 
    +    public int len() ;
    +
    +    /** Convert to a List */
    +    public default List<X> asList() {
    +        return new TupleList<>(this) ;
    +    }
    +
    +    /** stream */
    +    public default Stream<X> stream() { 
    +        return asList().stream() ;
    +    }
    +
    +    /** forEach */
    +    @Override
    +    public default void forEach(Consumer<? super X> action) { 
    +        asList().forEach(action) ;
    +    }
    +
    +    /** Copy the Tuple into the array */ 
    +    public default void copyInto(X[] array) {
    +        copyInto(array, 0, len());
    +    }
    +
    +    /** Copy the Tuple into the array */ 
    +    public default void copyInto(X[] array, int start) {
    +        copyInto(array, start, len());
    +    }
    +
    +    /** Copy the Tuple into the array */ 
    +    public void copyInto(X[] array, int start, int length) ;
    --- End diff --
    
    Isn't there a really obvious `default` here? Maybe it would be good to have a comment here explaining why impls need to do their own thing here...?


---
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-1107 : Improve Tuple.

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/115#discussion_r48558378
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/Tuple.java ---
    @@ -0,0 +1,73 @@
    +/*
    + * 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.List ;
    +import java.util.function.Consumer ;
    +import java.util.stream.Stream ;
    +
    +import org.apache.jena.atlas.lib.ArrayUtils ;
    +
    +/** A Tuple is the same class of item */
    +public interface Tuple<X> extends Iterable<X> {
    +    /** Get the i'th element, for i in the range 0 to len()-1 
    +     * @throws IndexOutOfBoundsException for i out of range 
    +     */
    +    public X get(int i) ;
    +
    +    /** length : elements are 0 to len()-1 */ 
    +    public int len() ;
    +
    +    /** Convert to a List */
    +    public default List<X> asList() {
    +        return new TupleList<>(this) ;
    +    }
    +
    +    /** stream */
    +    public default Stream<X> stream() { 
    --- End diff --
    
    Good 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-1107 : Improve Tuple.

Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:

    https://github.com/apache/jena/pull/115#discussion_r48498599
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/Tuple.java ---
    @@ -0,0 +1,73 @@
    +/*
    + * 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.List ;
    +import java.util.function.Consumer ;
    +import java.util.stream.Stream ;
    +
    +import org.apache.jena.atlas.lib.ArrayUtils ;
    +
    +/** A Tuple is the same class of item */
    --- End diff --
    
    This comment is a bit opaque. Does it mean that all items in the tuple are expected to have the same class?


---
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-1107 : Improve Tuple.

Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:

    https://github.com/apache/jena/pull/115#discussion_r48558360
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/Tuple.java ---
    @@ -0,0 +1,73 @@
    +/*
    + * 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.List ;
    +import java.util.function.Consumer ;
    +import java.util.stream.Stream ;
    +
    +import org.apache.jena.atlas.lib.ArrayUtils ;
    +
    +/** A Tuple is the same class of item */
    +public interface Tuple<X> extends Iterable<X> {
    +    /** Get the i'th element, for i in the range 0 to len()-1 
    +     * @throws IndexOutOfBoundsException for i out of range 
    +     */
    +    public X get(int i) ;
    +
    +    /** length : elements are 0 to len()-1 */ 
    +    public int len() ;
    +
    +    /** Convert to a List */
    +    public default List<X> asList() {
    +        return new TupleList<>(this) ;
    +    }
    +
    +    /** stream */
    +    public default Stream<X> stream() { 
    +        return asList().stream() ;
    +    }
    +
    +    /** forEach */
    +    @Override
    +    public default void forEach(Consumer<? super X> action) { 
    +        asList().forEach(action) ;
    +    }
    +
    +    /** Copy the Tuple into the array */ 
    +    public default void copyInto(X[] array) {
    +        copyInto(array, 0, len());
    +    }
    +
    +    /** Copy the Tuple into the array */ 
    +    public default void copyInto(X[] array, int start) {
    +        copyInto(array, start, len());
    +    }
    +
    +    /** Copy the Tuple into the array */ 
    +    public void copyInto(X[] array, int start, int length) ;
    --- End diff --
    
    I can move that (and iterator()) into the interface.  
    
    (Now TupleBase is needed (because you can't default methods from Object in an interface) some of all can go there if it keeps the source clean.)


---
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-1107 : Improve Tuple.

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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-1107 : Improve Tuple.

Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:

    https://github.com/apache/jena/pull/115#discussion_r48498210
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/lib/tuple/Tuple.java ---
    @@ -0,0 +1,73 @@
    +/*
    + * 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.List ;
    +import java.util.function.Consumer ;
    +import java.util.stream.Stream ;
    +
    +import org.apache.jena.atlas.lib.ArrayUtils ;
    +
    +/** A Tuple is the same class of item */
    +public interface Tuple<X> extends Iterable<X> {
    +    /** Get the i'th element, for i in the range 0 to len()-1 
    +     * @throws IndexOutOfBoundsException for i out of range 
    +     */
    +    public X get(int i) ;
    +
    +    /** length : elements are 0 to len()-1 */ 
    +    public int len() ;
    +
    +    /** Convert to a List */
    +    public default List<X> asList() {
    +        return new TupleList<>(this) ;
    +    }
    +
    +    /** stream */
    +    public default Stream<X> stream() { 
    --- End diff --
    
    `Iterable` has a `default` impl for `::spliterator` that could be used to make a `Stream`. I think that could avoid the extra object creation in `Tuple::asList`. That's how the `default` works for `Collection::stream`.


---
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.
---