2
\$\begingroup\$

In groovy I have a list of files A and also another list of files B. I want to remove all the files from A that have a filename that also occurs in the list of files B. I wrote the following code:

List<File> removeByName(final List<File> files, final List<File> removalFiles) {
    final Set<String> removalNames = removalFiles.collect { it.name }.toSet()
    files.findAll { ! removalNames.contains(it.name) }
}

Unfortunately I cannot use the remove method in the Collection class because two files are not equal even if they have the same file name. Are there any methods that I am missing which could be used to make that code cleaner?

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

Groovy's removeAll has an overload allowing use of a closure. This would enable you to completely ditch this method in favor of something like:

files.removeAll { removalFiles.collect { f -> f.name }.contains(it.name) } 

This unfortunately executes the transformation of removalFiles to removalNames for each item in files, which is undesirable. It is definitely easier, cleaner and most probably considerably faster to use your method.

\$\endgroup\$
4
\$\begingroup\$

Groovy is weakly typed, so you could save a some cruft using def for local variables. Typing can give nice hints (great on an API), but it can also be ignored, leading to code lies and confusion. Final is also a pretty pointless keyword in Groovy.

There's also a shortcut to collecting properties: the spread operator *, which allows you to do removalFiles*.name. It's also more groovy to cast to a set with as Set.

If you really need to do an in-place change, what you want to do is collect all the file objects from the files collection that you want to remove in order to remove them. This way, you are removing using the exact same object:

void removeByNameInPlace(def files, def removalFiles) {
    def removalNames = removalFiles*.name as Set
    files.removeAll( files.findAll { removalNames.contains(it.name) } )
}

However this isn't very groovy and it's typical to return modified copies of Collections instead. This makes your code pretty close to ideal, functionally speaking. With some stylistic changes to your original code:

List<File> removeByName(List<File> files, List<File> removalFiles) {
    def removalNames = removalFiles*.name as Set
    files.findAll { ! removalNames.contains(it.name) }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.