Discussion: easier Stream closing

[Remi Forax]

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 ?

Rémi

----- Original Message -----
> From: "Brian Goetz" <brian.goetz at oracle.com>
> To: "Tagir Valeev" <amaembo at gmail.com>, "core-libs-dev" <core-libs-dev at 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)