CODEWITHSUNDEEP
java
ab11
ab11

Reputation: 20090

How to concisely collect a stream to a map?

Given a list of directory names, I would like to produce a mapping of all files in the directories to their contents.

Any suggestions for a more explicit implementation?

    Map<File, String> map = new HashMap<>();

    Arrays.stream(filenames)
            .map(s -> FileUtils.listFiles(new File(rootdir, s), new RegexFileFilter(".*"), DirectoryFileFilter.DIRECTORY))
            .flatMap(Collection::stream)
            .forEach(f -> {
                try {
                    map.put(f, FileUtils.readFileToString(f));
                }
                catch (IOException e) {
                    e.printStackTrace();
                }
            });

    return map;

`

Upvotes: 3

Views: 103

Answers (3)

davidxxx
davidxxx

Reputation: 131326

Your way of doing is really not bad but I wonder whether in your case using streams is the best way to get a more explicit code.
The Aaron solution is a little bit straighter and more "stream way" as it collects the map as terminal operation but it also changes the original logic in case of thrown IOException.

A version without stream appears really straight :

Map<File, String> map = new HashMap<>();
for (String filename : filenames) {           
    for (File file : FileUtils.listFiles(new File(rootdir, filename), new RegexFileFilter(".*"), DirectoryFileFilter.DIRECTORY)) {
        try {
            map.put(file, FileUtils.readFileToString(file));
        } catch (IOException e) {
            // log exception
        }
    }
}

Edit : the solution of tsolakp with stream is really fine even if it stays less straight than the way using classic loops.

Here is a lighter version.
I think that using Optional internally to the stream is not required. It is "our" stream, so we master what we put in (here null for failure case).
In addition, including the code in the map() processing may seem less "functional" but it stays straight readable and I find it preferable to reading indirection. 

import java.util.AbstractMap.SimpleImmutableEntry;
// ...
Map<File, String> map = Arrays.stream(filenames)
        .flatMap( s -> FileUtils.listFiles( new File(rootdir, s), new RegexFileFilter(".*"), DirectoryFileFilter.DIRECTORY ).stream() )  
        .map( f -> {                    
                    try{ 
                        return new SimpleImmutableEntry<>(f, FileUtils.readFileToString(f) ); 
                    }catch(IOException e){
                        //log exception
                        return null; 
                    }                        
            }
         )
        .filter(Objects::nonNull)
        .collect(Collectors.toMap(SimpleImmutableEntry::getKey, SimpleImmutableEntry::getValue));

Upvotes: 3

Aaron
Aaron

Reputation: 24802

You should use Collectors.toMap :

Map<File, String> map = Arrays.stream(filenames)
    .map(s -> FileUtils.listFiles(new File(rootdir, s), new RegexFileFilter(".*"), DirectoryFileFilter.DIRECTORY))
    .flatMap(Collection::stream)
    .collect(Collectors.toMap(
        Function.identity(),
        f-> {
            try { return FileUtils.readFileToString(f); }
            catch (IOException ioe) { return null; }
        }
    ));

Its first parameter is the key mapping function. The unmodified files should be used as keys, so we can use the identity function.
Its second parameter is the value mapping function, where we map the Files to their content.

Note that my value mapping function will produce null values and won't output any error when a file's content can't be read. Consider using one of the other answers unless you're confident such IOException will never happen.

Upvotes: 3

tsolakp
tsolakp

Reputation: 5948

"more explicit implementation" is really opinion based. For me using plain loop like @davidxxx did is the preferred way. You can of course use Java streams too but their lack of handling exceptions will make your code with try/catch statement ugly. One option is to move the try/catch into separate method or utility class and keep the stream expression cleaner with something like this:

Map<File, String> map = Arrays.stream(filenames)
.flatMap( s -> FileUtils.listFiles( new File(rootdir, s), new RegexFileFilter(".*"), DirectoryFileFilter.DIRECTORY ).stream() )  
.map( f -> new AbstractMap.SimpleImmutableEntry<>(f, read(f) ) )
.filter( e -> e.getValue().isPresent() )
.collect( Collectors.toMap( e -> e.getKey(), e -> e.getValue().get() ) );

private Optional<String> read(File f){
    try{ 
        return Optional.of( FileUtils.readFileToString(f) ); 
    }catch(IOException e){
        //log exception
        return Optional.empty();
    }
}

Upvotes: 1

Related Questions