peterdemaeyer commented on code in PR #827:
URL: https://github.com/apache/commons-io/pull/827#discussion_r2791758809
##########
src/main/java/org/apache/commons/io/file/CopyDirectoryVisitor.java:
##########
@@ -53,7 +54,7 @@ private static CopyOption[] toCopyOption(final CopyOption...
copyOptions) {
* @param copyOptions Specifies how the copying should be done.
*/
public CopyDirectoryVisitor(final PathCounters pathCounter, final Path
sourceDirectory, final Path targetDirectory, final CopyOption... copyOptions) {
- super(pathCounter);
+ super(pathCounter, TrueFileFilter.INSTANCE, TrueFileFilter.INSTANCE);
Review Comment:
If you're worried about this API change, I propose to revert it and call the
other constructor in the only the place where it's called, which is
`PathUtils.copyDirectory`, line 417.
Beyond the scope of the issue at hand, I'd like to also share with you my
impressions while working on the fix.
The reason we're even discussing this is because `CopyDirectoryVisitor` is
on the API, which maybe should not have been the case. As a user and fixer of
this bug, my focus is to get `copyDirectory` working intuitively. I am familiar
the JDK and its `FileVisitor` API, it is the API I would have liked to use.
Instead, I was burdened with having to understand, implement and extend
`CopyDirectoryVisitor` < `CountingPathVisitor` < `SimplePathVisitor` <
`PathVisitor`/`SimpleFileVisitor`, which does a bunch of stuff I don't need,
such as counting bytes and filtering.
Given that `CopyDirectoryVisitor` is only used for the implementation of
`PathUtils.copyDirectory`, it looks to me like an implementation detail that
leaked onto the API.
Don' get me wrong, I can see a use case for counting bytes and filtering,
but I would have offered them as independent `ByteCountingFileVisitor` and
`FilteringFileVisitor` decorators, for users to only use them if and when they
need them. This is an example of where applying the design principle "prefer
composition over inheritance" would have resulted in better separation of
concerns and a more flexible API.
--
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]
