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/07/29 16:45:37 UTC

[GitHub] jena pull request: Richer Javadocs for Iter and ExtendedIterator

GitHub user ajs6f opened a pull request:

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

    Richer Javadocs for Iter and ExtendedIterator

    

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

    $ git pull https://github.com/ajs6f/jena DocsForIterAndExtendedIterator

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

    https://github.com/apache/jena/pull/90.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 #90
    
----
commit 7730fb47020655369152da065b6c744484b3f15c
Author: ajs6f <aj...@virginia.edu>
Date:   2015-07-29T14:44:27Z

    Richer Javadocs for Iter and ExtendedIterator

----


---
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: Richer Javadocs for Iter and ExtendedIterator

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

    https://github.com/apache/jena/pull/90#discussion_r35871606
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/iterator/Iter.java ---
    @@ -29,6 +29,14 @@
     import org.apache.jena.atlas.lib.Closeable ;
     import org.apache.jena.atlas.lib.Sink ;
     
    +/**
    + * Iter provides utilities for working with {@link Iterator}s.
    + *
    + * Iter should never be used as a return type or parameter type in the public contract of a class. It is only to be used
    + * inside implementation code and is instantiated only to allow method-chaining as part of a calculation.
    --- End diff --
    
    IMO `Iter` is better than `ExtendedIterator` (more general, separated out) and should be used on general iterators. `ExtendedIterator` exists in jena-core and Graph API only and as a Triple iterator. 
    
    As ExtendedIterator is exposed, it can't be changed. 
    
    So if and where working on triple iterators out of Graph, use ExtendedIterator, else Iter.
    



---
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: Richer Javadocs for Iter and ExtendedIterator

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

    https://github.com/apache/jena/pull/90#discussion_r35871940
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/iterator/Iter.java ---
    @@ -29,6 +29,14 @@
     import org.apache.jena.atlas.lib.Closeable ;
     import org.apache.jena.atlas.lib.Sink ;
     
    +/**
    + * Iter provides utilities for working with {@link Iterator}s.
    + *
    + * Iter should never be used as a return type or parameter type in the public contract of a class. It is only to be used
    + * inside implementation code and is instantiated only to allow method-chaining as part of a calculation.
    --- End diff --
    
    This is exactly what I needed. Sorry to have bugged you so much about it, but I really think this deserves to be written down. I will correct this PR to advise just what you just did.
    
    I am a little surprised that Jena's policy is that once a type is exposed, it can never be changed. That seems extraordinarily restrictive. But that's a completely different question.


---
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: Richer Javadocs for Iter and ExtendedIterator

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

    https://github.com/apache/jena/pull/90#issuecomment-129442497
  
    Since commit https://github.com/apache/jena/commit/fd5a94ae61bcd32d2a82cebb6d088d0d81f1da39 contains literally none of the usage advice that was the only purpose of this PR and merely reformats the extant comments, this PR should probably be marked "rejected", not "closed with".


---
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: Richer Javadocs for Iter and ExtendedIterator

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

    https://github.com/apache/jena/pull/90#discussion_r35807971
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/iterator/Iter.java ---
    @@ -29,6 +29,14 @@
     import org.apache.jena.atlas.lib.Closeable ;
     import org.apache.jena.atlas.lib.Sink ;
     
    +/**
    + * Iter provides utilities for working with {@link Iterator}s.
    + *
    + * Iter should never be used as a return type or parameter type in the public contract of a class. It is only to be used
    + * inside implementation code and is instantiated only to allow method-chaining as part of a calculation.
    --- End diff --
    
    Yes, that's why the note is useful. If `Iter` actually imposed that in the code there would be no need for documentation.
    
    Is there in fact any difference between `Iter` and `ExtendedIterator`? They are both subtypes of `java.util.Iterator` that provide convenient methods, with the functionality of `Iter` entirely overlapping that of `ExtendedIterator`. I've tried repeatedly to discover this information, and my experience shows that it is quite possible to confuse them. You were quite explicit in explaining to me in connection with JENA-966 that they are not to be used in the same circumstances, and all I'm trying to do here is determine what those circumstances actually are and record them.


---
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: Richer Javadocs for Iter and ExtendedIterator

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

    https://github.com/apache/jena/pull/90#discussion_r35813136
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/iterator/Iter.java ---
    @@ -29,6 +29,14 @@
     import org.apache.jena.atlas.lib.Closeable ;
     import org.apache.jena.atlas.lib.Sink ;
     
    +/**
    + * Iter provides utilities for working with {@link Iterator}s.
    + *
    + * Iter should never be used as a return type or parameter type in the public contract of a class. It is only to be used
    + * inside implementation code and is instantiated only to allow method-chaining as part of a calculation.
    --- End diff --
    
    What do you mean by "difference"?  Can they do the same things? Yes.  Why are there two? History (Iter came along later for use in SDB and TDB and works on iterators; ExtendedIterator works on ExtendedIterator iterators and appears in an important API).


---
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: Richer Javadocs for Iter and ExtendedIterator

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

    https://github.com/apache/jena/pull/90#discussion_r35869523
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/iterator/Iter.java ---
    @@ -29,6 +29,14 @@
     import org.apache.jena.atlas.lib.Closeable ;
     import org.apache.jena.atlas.lib.Sink ;
     
    +/**
    + * Iter provides utilities for working with {@link Iterator}s.
    + *
    + * Iter should never be used as a return type or parameter type in the public contract of a class. It is only to be used
    + * inside implementation code and is instantiated only to allow method-chaining as part of a calculation.
    --- End diff --
    
    What I meant by "difference" was context of use. In other words, what I have been trying to get at with this conversation, as I have written, is the answer to "When should one use `Iter`, and when should one use `ExtendedIterator`?" I tried to use `Iter` inside an `ExtendedIterator` during JENA-966 and you explained that this was wrong, and I am trying to get Javadocs into these classes so that other people don't have the same confusion. 
    
    I still have no idea when to use either. If the difference is purely historical, then there is no inherent need for both, and perhaps I should open a ticket for a slow, careful, un-disruptive migration towards a single type for these purposes?


---
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: Richer Javadocs for Iter and ExtendedIterator

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

    https://github.com/apache/jena/pull/90#discussion_r35807211
  
    --- Diff: jena-base/src/main/java/org/apache/jena/atlas/iterator/Iter.java ---
    @@ -29,6 +29,14 @@
     import org.apache.jena.atlas.lib.Closeable ;
     import org.apache.jena.atlas.lib.Sink ;
     
    +/**
    + * Iter provides utilities for working with {@link Iterator}s.
    + *
    + * Iter should never be used as a return type or parameter type in the public contract of a class. It is only to be used
    + * inside implementation code and is instantiated only to allow method-chaining as part of a calculation.
    --- End diff --
    
    Iter itself does not impose that in its contract.  It's the rest of jena (the users of Iter) that choose to return "Iterator" not "Iter".  c.f. Map and HashMap. 
    
    "Iter is not usually used as a return type ..." which is more advice that instruction.  If some code wishes to expose method-chaining then no problem.


---
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: Richer Javadocs for Iter and ExtendedIterator

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

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


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