Improve Cursor Closeable behavior #1494
Conversation
…erride close() to no longer throw IOException
|
@hutteman Thanks for your contribution!! I think this PR is good and reasonable improvement. However this changes has cases that broken backward-compatibility at compile time. Therefore I think PR should merge at next minor(3.6) or major(4.0) version-up.
e.g. @Test
public void closeTryWithResourcesWithCatch() {
try (Cursor<Object> c = mapper.select()) {
for (Object o : c) {
c.isOpen();
}
} catch (IOException e) { // this code is broken
e.printStackTrace();
}
}
@Test
public void closeTryWithResourcesWithThrows() throws IOException { // this code is not broken
try (Cursor<Object> c = mapper.select()) {
for (Object o : c) {
c.isOpen();
}
}
} |
|
Thank you, @hutteman ! @kazuki43zoo , Anyway, as you suggested, let's plan this for the next major release for now. |
|
How about thinking about a 3.6.x version? It's been more than 4 years since version 3.5.x @harawata @kazuki43zoo |
|
Yes, I also think upgrading to Java 11 would be good, are there any features rodmaps that we can think of for this new version? I think thinking about Ahead-of-Time (AOT) in a 3.6.x or 4 version would be a good idea at least an initial implementation is a move that many are making |
|
@GeorgeSalu Head over to discussions tab, I have created a discussion for mybatis 3.6.0. Let that server as the discussion point for any wants to the framework. I have listed this specific item as part of that but feel free to bring up AOT there. I've not worked with that myself so we would probably expect community help in laying out the roadmap and contributing to it. Thanks. |
# Conflicts: # src/test/java/org/apache/ibatis/submitted/cursor_simple/CursorSimpleTest.java
|
It took a while 😉, but it has been merged. |
See https://groups.google.com/forum/#!topic/mybatis-user/PzTPldYdThA
The
Cursorinterface extendsCloseable, enabling its usage in a try-with-resources construct.Unfortunately by not explicitly overriding
Closeable.close(), any try-with-resources over aCursorforces developers to have to catchIOException, even though this isn't thrown byDefaultCursor, nor would it make much sense in any futureCursorimplementation.This change adds the below to the
Cursorinterface to make them easier to work with in try-with-resources :It also changes
Cursorto overrideAutoCloseablerather thanCloseable, as suggested by Iwao on the google groups thread.Technically, this is a breaking change, as existing code that either explicitly or implicitly calls
Cursor.close()and currently catchesIOException(even though it will never be thrown) will likely need to remove that catch-block or fail compilation. This is an easy change to make during the upgrade to the new version however, and will simplify their code and that of any future users ofCursor.By the way the javadocs for
AutoCloseable.close()explicitly mention this: