On 11/03/2021 22:51, Claude Warren wrote:
My thought in all this was that the try with resources is now standard in
Java so we should try to support it.

Try-with-resources is primarily for external resources.

Streams are the way for programming.

They are less general that iterators, iterator are very low level - when the stream pipeline starts, it executes and the end is known. No single stepping.

Hence ExtendedIterator.forEach. (added after a previous close-eIterator discussion).

The obvious incremental addition is a default method that is "process iterator, return calculation". See "iterRtn" though it feels a bit clunky still.

We have use of iterators for "possible multiple results" and how multiple property-values are handled is the API question. Closability is a tool, not a solution.

Example: Stream.findFirst isn't a pattern to test if there were unexpected more than one occurrence.


Perhaps it makes sense to do the change in V4.
We can clean up our code so that there are no warnings.

Since the issues are warnings only, it should be manageable for our users.

As I've said before, I think that tweaking the API has limited benefit.

Starting a new one without needing to be concerned with the incremental migration seems to me to be easier.

Streams being one example of a new API.

    Andy


Claude

On Thu, Mar 11, 2021 at 10:48 AM Andy Seaborne <a...@apache.org> wrote:

I'm not clear which problem we are discussing:

1/ Ensuring inside jena-permissions that iterators are properly closed.

2/ Ensuring use code using jena-permissions (i.e
SecuredGraph/SecuredModel by name) closes iterators properly.

3/ Ensuring app code that uses the Graph or Model interface (and so may
be Secured* but it does not know) closes iterators properly.

If (3) isn't any change going to impact existing use code?

What's more, it does not seem to work

On 10/03/2021 21:56, Claude Warren wrote:
I think that if the solution was any simpler it would have bit me.

Change ClosableIterator<T> to extend AutoCloseable as well as Iterator.
mark ClosableIterator.close() with an @Override annotation and done.

While Autoclosable throws an exception the ClosableIterator does not so
the
try with resources works.

I tried this and then changed
FinderUtils.contains() to be coded as

@Override
          public boolean contains(TriplePattern pattern) {
              try (ClosableIterator<Triple> it = find(pattern))
              {
              return it.hasNext();
              }
          }

and the IDE (Eclipse) does not complain.  So I think it will just work.

because writing

public boolean contains(TriplePattern pattern) {
        ExtendedIterator<Triple> it = find(pattern);
        try {
           return it.hasNext();
        } finally { it.close(); }
}

is accident-prone?


I changed ClosableIterator as you describe and got hundreds of warnings
from the codebase, and it will happen to user code.

1/
A warning happens on code that is currently correctly written old style as

      ExtendedIterator<> iter =
      try {
        ...
      } finally { iter.close(); }

Eclipse "quick-fix" does not do a good job on this.

The other "old style" code is where the iterator runs the the end
(exceptions aside).

2/
Sometimes there isn't a warning!

Warning: as expected:
      public static void main(String...a) throws FileNotFoundException {
          InputStream in = new FileInputStream("");
      }

No warning:  !!!!
      static ExtendedIterator<Triple> foo() { return null; }
      public static void main(String...a) {
          ExtendedIterator<Triple> iter = foo();
      }

Warning:
      static ExtendedIterator<Triple> foo() { return null; }
      public static void main(String...a) {
          ExtendedIterator<Triple> iter = foo();
          iter.close();
      }

Weird.
That somewhat negates the utility.

      Andy



Reply via email to