Cursor.close

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Cursor.close

Luke Hutteman
The Cursor interface extends Closeable, enabling its usage in a try-with-resources construct.

Unfortunately by not explicitly overriding Closeable.close(), any try-with-resources over a Cursor forces developers to have to catch IOException, even though this isn't thrown by DefaultCursor, nor would it make much sense in any future Cursor implementation.

Could the below be added to the Cursor interface to make them easier to work with in try-with-resources? :

    @Override
    void close();


Technically, adding this would be a breaking change, as existing code that either explicitly or implicitly calls Cursor.close() and currently catches IOException (even though it will never be thrown) will likely need to remove that catch-block or fail compilation. This would be an easy change to make during the upgrade to the new version however, and would simplify their code and that of any future users of Cursor.

By the way the javadocs for AutoCloseable.close() explicitly mention this:

     * <p>While this interface method is declared to throw {@code
     * Exception}, implementers are <em>strongly</em> encouraged to
     * declare concrete implementations of the {@code close} method to
     * throw more specific exceptions, or to throw no exception at all
     * if the close operation cannot fail.



--
You received this message because you are subscribed to the Google Groups "mybatis-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Cursor.close

Iwao AVE!
Hi Luke,

Thank you for the suggestion!
It seems to make sense.
We should also change Cursor to extend AutoCloseable instead of Closeable, probably.
(the Cursor had to support Java 6 when it was implemented, but not anymore)

Could you send a pull request?
If you're busy, it's fine and I'll do it when I have time. :)

Regards,
Iwao


On Sat, Mar 2, 2019 at 11:52 AM Luke Hutteman <[hidden email]> wrote:
The Cursor interface extends Closeable, enabling its usage in a try-with-resources construct.

Unfortunately by not explicitly overriding Closeable.close(), any try-with-resources over a Cursor forces developers to have to catch IOException, even though this isn't thrown by DefaultCursor, nor would it make much sense in any future Cursor implementation.

Could the below be added to the Cursor interface to make them easier to work with in try-with-resources? :

    @Override
    void close();


Technically, adding this would be a breaking change, as existing code that either explicitly or implicitly calls Cursor.close() and currently catches IOException (even though it will never be thrown) will likely need to remove that catch-block or fail compilation. This would be an easy change to make during the upgrade to the new version however, and would simplify their code and that of any future users of Cursor.

By the way the javadocs for AutoCloseable.close() explicitly mention this:

     * <p>While this interface method is declared to throw {@code
     * Exception}, implementers are <em>strongly</em> encouraged to
     * declare concrete implementations of the {@code close} method to
     * throw more specific exceptions, or to throw no exception at all
     * if the close operation cannot fail.



--
You received this message because you are subscribed to the Google Groups "mybatis-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "mybatis-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Cursor.close

Luke Hutteman
Thanks for the feedback; I just created a pull request for this at https://github.com/mybatis/mybatis-3/pull/1494 , also including your suggestion of extending AutoCloseable instead of Closeable.

On Wednesday, March 6, 2019 at 2:14:00 PM UTC-5, Iwao AVE! wrote:
Hi Luke,

Thank you for the suggestion!
It seems to make sense.
We should also change Cursor to extend AutoCloseable instead of Closeable, probably.
(the Cursor had to support Java 6 when it was implemented, but not anymore)

Could you send a pull request?
<a href="https://github.com/mybatis/mybatis-3/" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fmybatis%2Fmybatis-3%2F\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFQYuWFZosRkw6UOTFXYRW8flEJ9A&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fmybatis%2Fmybatis-3%2F\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFQYuWFZosRkw6UOTFXYRW8flEJ9A&#39;;return true;">https://github.com/mybatis/mybatis-3/
If you're busy, it's fine and I'll do it when I have time. :)

Regards,
Iwao


On Sat, Mar 2, 2019 at 11:52 AM Luke Hutteman <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="8uArX3DLAwAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">hutt...@...> wrote:
The Cursor interface extends Closeable, enabling its usage in a try-with-resources construct.

Unfortunately by not explicitly overriding Closeable.close(), any try-with-resources over a Cursor forces developers to have to catch IOException, even though this isn't thrown by DefaultCursor, nor would it make much sense in any future Cursor implementation.

Could the below be added to the Cursor interface to make them easier to work with in try-with-resources? :

    @Override
    void close();


Technically, adding this would be a breaking change, as existing code that either explicitly or implicitly calls Cursor.close() and currently catches IOException (even though it will never be thrown) will likely need to remove that catch-block or fail compilation. This would be an easy change to make during the upgrade to the new version however, and would simplify their code and that of any future users of Cursor.

By the way the javadocs for AutoCloseable.close() explicitly mention this:

     * <p>While this interface method is declared to throw {@code
     * Exception}, implementers are <em>strongly</em> encouraged to
     * declare concrete implementations of the {@code close} method to
     * throw more specific exceptions, or to throw no exception at all
     * if the close operation cannot fail.



--
You received this message because you are subscribed to the Google Groups "mybatis-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="8uArX3DLAwAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">mybatis-user...@googlegroups.com.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "mybatis-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.