My thought in all this was that the try with resources is now standard in
Java so we should try to support it.

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.

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

-- 
I like: Like Like - The likeliest place on the web
<http://like-like.xenei.com>
LinkedIn: http://www.linkedin.com/in/claudewarren

Reply via email to