> At that point, the real question is why not call close() at the end of all > terminal operations by wrapping each implementation in a try-with-resources ?
Now, it will be incompatible change. Also, this won't work with iterator() and spliterator() terminal operations. I believe it's best described by Brian in this SO answer: https://stackoverflow.com/a/34073306/4856258 Quoted here, for convenience: > Yes, this was a deliberate decision. We considered both alternatives. > > The operating design principle here is "whoever acquires the resource should > release the resource". Files don't auto-close when you read to EOF; we expect > files to be closed explicitly by whoever opened them. Streams that are backed > by IO resources are the same. > > Fortunately, the language provides a mechanism for automating this for you: > try-with-resources. Because Stream implements AutoCloseable, you can do: > > try (Stream<String> s = Files.lines(...)) { > s.forEach(...); > } > > The argument that "it would be really convenient to auto-close so I could > write it as a one-liner" is nice, but would mostly be the tail wagging the > dog. If you opened a file or other resource, you should also be prepared to > close it. Effective and consistent resource management trumps "I want to > write this in one line", and we chose not to distort the design just to > preserve the one-line-ness. With best regards, Tagir Valeev. > > Rémi > > ----- Original Message ----- > > From: "Brian Goetz" <brian.go...@oracle.com> > > To: "Tagir Valeev" <amae...@gmail.com>, "core-libs-dev" > > <core-libs-dev@openjdk.java.net> > > Sent: Lundi 27 Septembre 2021 22:41:00 > > Subject: Re: Discussion: easier Stream closing > > > In Java 8, I think we were reluctant to lean on the idiom of "pass me a > > lambda and I'll pass it the confined data"), because Java developers > > were already struggling to understand lambdas. But now that we're > > mostly over that hurdle, API points that accept Consumer<Something> are > > a powerful way to gain confinement (as we've seen in StackWalker, and > > also in the explorations of ScopeLocal in Loom.) So I have no objection > > to this idiom. > > > > I have some concern that putting these side-by-side with existing > > Files.walk(...) methods will be a source of confusion, creating two > > different ways to do the same thing. > > > > I would rather not add anything new to BaseStream; it was a mistake to > > expose at all, and I'd rather see it deprecated than add more to it. > > However, adding it individually to the Stream implementations is > > equivalent, and doesn't have this problem. > > > > The consumeAndClose approach is clever, in that it adds one API point > > that works for all streams, rather than having to add a new API point > > for every factory of a closeable stream; on the other hand, it is > > dramatically less discoverable, and so requires more education to get > > people to use it (and, as you say, has the exception problem.) > > > > On 9/26/2021 5:27 AM, Tagir Valeev wrote: > >> Hello! > >> > >> With current NIO file API, a very simple problem to get the list of > >> all files in the directory requires some ceremony: > >> > >> List<Path> paths; > >> try (Stream<Path> stream = Files.list(Path.of("/etc"))) { > >> paths = stream.toList(); > >> } > >> > >> If we skip try-with-resources, we may experience OS file handles leak, > >> so it's desired to keep it. Yet, it requires doing boring stuff. In > >> this sense, classic new File("/etc").list() was somewhat more > >> convenient (despite its awful error handling). > >> > >> I like how this problem is solved in StackWalker API [1]: it limits > >> the lifetime of the Stream by requiring a user-specified function to > >> consume it. After the function is applied, the stream is closed > >> automatically. We could add a similar overload to the Files API. As an > >> additional feature, we could also translate all UncheckedIOException's > >> back to IOException for more uniform exception processing: > >> > >> /** > >> * @param dir The path to the directory > >> * @param function function to apply to the stream of directory files > >> * @param <T> type of the result > >> * @return result of the function > >> * @throws IOException if an I/O error occurs when opening the directory, > >> or > >> * UncheckedIOException is thrown by the supplied function. > >> */ > >> public static <T> T list(Path dir, Function<? super Stream<Path>, ? > >> extends T> function) throws IOException { > >> try (Stream<Path> stream = Files.list(dir)) { > >> return function.apply(stream); > >> } > >> catch (UncheckedIOException exception) { > >> throw exception.getCause(); > >> } > >> } > >> > >> In this case, getting the List of all files in the directory will be > >> as simple as > >> > >> List<Path> paths = Files.list(Path.of("/etc"), Stream::toList); > >> This doesn't limit the flexibility. For example, if we need only file > >> names instead of full paths, we can do this: > >> List<Path> paths = Files.list(Path.of("/etc"), s -> > >> s.map(Path::getFileName).toList()); > >> > >> Alternatively, we could enhance the BaseStream interface in a similar > >> way. It won't allow us to translate exceptions, but could be useful > >> for every stream that must be closed after consumption: > >> > >> // in BaseStream: > >> > >> /** > >> * Apply a given function to this stream, then close the stream. > >> * No further operation on the stream will be possible after that. > >> * > >> * @param function function to apply > >> * @param <R> type of the function result > >> * @return result of the function > >> * @see #close() > >> */ > >> default <R> R consumeAndClose(Function<? super S, ? extends R> function) { > >> try(@SuppressWarnings("unchecked") S s = (S) this) { > >> return function.apply(s); > >> } > >> } > >> > >> The usage would be a little bit longer but still more pleasant than > >> explicit try-with-resources: > >> > >> List<Path> list = > >> Files.list(Path.of("/etc")).consumeAndClose(Stream::toList); > >> > >> On the other hand, in this case, we are free to put intermediate > >> operations outside of consumeAndClose, so we won't need to nest > >> everything inside the function. Only terminal operation should be > >> placed inside the consumeAndClose. E.g., if we need file names only, > >> like above, we can do this: > >> > >> List<Path> list = > >> Files.list(Path.of("/etc")).map(Path::getFileName).consumeAndClose(Stream::toList); > >> > >> What do you think? > >> > >> > >> [1] > > > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#walk(java.util.function.Function)
