laeubi commented on code in PR #913:
URL: https://github.com/apache/maven/pull/913#discussion_r1081641445
##########
maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenWorkspaceReader.java:
##########
@@ -28,4 +29,20 @@
public interface MavenWorkspaceReader extends WorkspaceReader {
Model findModel(Artifact artifact);
+
+ /**
+ * List all available artifacts this workspace repository manages.
+ *
+ * @return a stream of artifacts in no particular order
+ * @since 3.9.0
+ */
+ Stream<Artifact> listArtifacts();
Review Comment:
Caching is not a concern of the API. Using streams allows implements to
cache (as they can be sure no one can ever modify the underlying collection,
what ever type and whatever technique). so this is really not an argument.
If I need to browse them twice (why?) I can again make a collection, I even
need to do so, because I can not assume it is always stable (e.g. modified)
unless I copy it (or the calling method copies it) so this is really not an
advantage at all.
> So at the end from a caller perspective the collection usage is lighter in
terms of contract and compatible with streams (.stream()).
I don't think so, and of course stream is compatible with collection as well
(.toList()) so this also is just not really an argument.
> now if you explicit you must enforce close
Sorry the statement of that one **must** close a stream was brought up by
you and I proved its wrong, the API clearly state that closing is not required
except for very special (and documented) cases, and even there it is documented
that one _should_ close the stream see:
https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#lines-java.nio.file.Path-java.nio.charset.Charset-
> If timely disposal of file system resources is required, the
try-with-resources construct should be used to ensure that the stream's
[close](https://docs.oracle.com/javase/8/docs/api/java/util/stream/BaseStream.html#close--)
method is invoked after the stream operations are completed.
> now if you explicit you must enforce close - which is the current PR
contract since everything is abstracted and can be backed by a filesystem by
contract - it is fine too, just not as convenient IMHO.
so this whole "close" story is completely unrelated here and is just
confusing, there are literally no code except the I/O cases where it is
recommended, and you won't find code that closes simple streams "just in
case"... and even if an implementation might has requirements for close it
still can collect everything, close the stream and return a "save" stream to
the caller, but that's nothing an API must take care of.
> Key decision point for me is: does the data fit in mem: obviously yes it
is in maps already, so no need to stream it IMHO.
This is obviously not a decisions point, because then most of the time no
one would ever use streams as most of them "fit in memory". And even if the
current one is held in memory this won't necessary be the case for other
implementations of this and it might be costly to collect everything, it even
might delay thing (just think about maven would resolve and download **all**
artifacts before the progress is reported in one big bunch, even though it is
same as fast, you will want a stream of messages as they happen)
Also please look at the `MavenChainedWorkspaceReader` implementation, that
currently can simply return the stream from inner providers, combine them, and
even can make sure they are still unique, while a collection approach would
require:
- Each implementation must collect everything eager (even if not used at all)
- It must create a copy (if dynamic) or read only collection
- The chained reader must again maintain own collections and add all element
it collects again, making it unique e.g. by a set (additional efforts) that
includes traversing all object
- The caller most likely will not just store the collection but iterate it
(foreach or stream) and thus must traverse all objects again.
Because of this, I'm strongly convinced that Streams are superior in this
case (and even others), are a more modern way of "collections" and are much
more flexible anyways, for everyone that is more used to collections they can
easily be transformed into such.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
