DEV Community

Cover image for Think twice before using a Supplier
Bastien Helders
Bastien Helders

Posted on

Think twice before using a Supplier

Supply and demand

At the end of my last article, we had this piece of non-functioning code, which tried to replace placeholder string from a first list with strings from a second list:

private static void populateTemplateFile(ArrayList<String> sKeys, ArrayList<String> sVals) {
    File file = new File(TEMPLATE_FILE);
    File trgFile = new File(TARGET_FILE);
    Stream<String> lines = null;

    try
    {
        lines = Files.lines(Paths.get(file.toURI()));

        //Newline for readability, this is only one line.
        Stream<String> replaced = lines
            .map(line -> {
                for(int iCtr=0; iCtr<sKeys.size(); iCtr++)
                    line.replaceAll(sKeys.get(iCtr), sVals.get(iCtr));
                    return line;});
        lines.close();
        Files.write(Paths.get(trgFile.toURI()), replaced.toString().getBytes());
    }
    catch (IOException e)
    {
       log.error("Exception while trying to create " + TARGET_FILE, e);
    }
    finally {
        lines.close();
    }
}
Enter fullscreen mode Exit fullscreen mode

I concluded the article that although I tried to use Stream, the fact that it is a double loop would make this case a poor candidate for what I wanted to do.

But then it came to me that for my first example, I used a Supplier to access the stream twice, why not n time? It would enable me to extract the for loop out of the map call and instead, I would replace the value in the lines for each of the element of the keys list.

It sounded so simple until I realized it wouldn't work. You see, when the Supplier supplies you the line of the file, it supplies you the lines as they were untouched. This is fine when you don't plan on modifying the values, but this is another story when you are doing replacements. In the end, only one replacement would have been made: the last one.

The Keys to success

However, all hopes were not lost. You see, at the same time I was trying in vain to use a Supplier, Lluís Josep Martinez pointed me to a way to write the output file as a stream.

I was at first skeptical, but he seemed sure of himself that it would be the solution. So I searched again for a solution.

First, it seemed sensible, as Lluís suggested, to use a Map as parameter of the method instead of two Lists. And make the parameter name more explicit.

private static void populateTemplateFile(Map<String,String> substitutionDictionary) {
Enter fullscreen mode Exit fullscreen mode

But it still didn't help me with my Supplier issue. So I tried to take another perspective. You see, my issue is that I can seem to find an adequate solution to loop through the keys and trying to replace them in the lines.

So what if I looped through the lines first instead? But then I'll get back to the issue of the for loop in the map call again. Unless I get rid of the second loop. You see, as a PHP dev, I used the in_array() function a lot to see if a string was contained in an Array. Granted, the lines containing the information contains more than only the key information (i.e. the placeholders) I was searching, but in a similar fashion, I could check if any of the keys was contained in a file line.

Function<String,String> replacePlaceholders = (line) -> {
    Set<String> keySet = substitutionDictionary.keySet();
    Optional<String> key = keySet.stream()
                                 .filter(line::contains)
                                 .findAny();

    if(key.isPresent()) {
        return line.replace(key.get(), substitutionDictionary.get(key.get()));
    }

    return line;
}

Enter fullscreen mode Exit fullscreen mode

And then I just needed to apply the code found on the link provided by Lluís.

try (Stream<String> lines = Files.lines(Paths.get(file.toURI()));
     PrintWriter output = new PrintWriter(TARGET_FILE, "UTF-8"))
{

    lines.map(s -> replacePlaceholders.apply(s))
         .forEachOrdered(output::println);
    output.close();
}
Enter fullscreen mode Exit fullscreen mode

In fact, using a Function to do data manipulation is so potent, that I found a similar solution for someone else's problem.

Leaky Supplier

To come back to the piece of code which inspired me to write my first article, I was glad, as it was working and got me the result I wanted. That is until I got a java.nio.file.FileSystemException telling me that there were too many open files... It turned out I was leaking file handles and I had to ensure that those handles would be closed. So I went ahead and used a try-with-resource as suggested.

Supplier<Stream<String>> linesSupplier = () -> {
    try(Stream<String> lines = Files.lines(Paths.get(file.toURI())))
    {
        return lines;
    }
    catch (IOException e)
    {
        log.error("Error while supplying file " + file.getName(), e);
    }
    return null;
};
Enter fullscreen mode Exit fullscreen mode

I got disappointed real fast, as it threw me an IllegalStateException as it did before I used a Supplier. And rightly so, as the Supplier closes the Stream it returned before I could use it.

Again, I had to change perspective. What if I stored all the values at once. And it is possible if I store the Predicate in a list and do all the filtering sequentially before the Stream closes.

List<String> values = null;
List<Predicate<String>> filters = Arrays.asList(sourcePattern.asPredicate(), contentPattern.asPredicate());
String source = "";
String content = "";

try (Stream<String> lines = Files.lines(Paths.get(file.toURI())))
{
    values = lines.filter(line -> filters.stream()
                                      .anyMatch(f -> f.test(line)))
                  .collect(Collectors.toList());
    source = values.get(0);
    content = values.get(1);
}
catch (IOException e)
{
    log.error("Error while filtering " + file.getName(), e);
}
Enter fullscreen mode Exit fullscreen mode

I can then use the values I got to my heart content. Yes, I've been said that I could have transferred the responsibility to close the Stream to the line of code which called the Supplier, but I think that in the end, I found a better way.

Parting thoughts

  • There is no doubt a good use for a Supplier, but until now, I didn't find one. So next time you try to use one, because you want to access your Stream more than once, ask yourself: "Can't I instead get all I need from the Stream at once?"
  • If you get stump by a problem, try to change perspective? More often than not, it helps.

Until the next time I need to change my point of view.

Top comments (2)

Collapse
 
evanoman profile image
Evan Oman • Edited

Similar to your solution you can compose all of your replace applications together into a single Function<String, String>

private static void populateTemplateFile(Map<String, String> replacementMap)
{
    File file = new File(TEMPLATE_FILE);

    /* Combine all of the replacements into a single function */
    Function<String, String> replacementFunction = replacementMap
            .entrySet()
            .stream()
            .map(kv -> (Function<String, String>) s -> 
                                s.replace(kv.getKey(),kv.getValue()))
            .reduce(Function.identity(), Function::andThen);

    try (Stream<String> lines = Files.lines(Paths.get(file.toURI()));
        PrintWriter output = new PrintWriter(TARGET_FILE, "UTF-8"))
    {
        lines.map(replacementFunction).forEachOrdered(output::println);
    }
    catch (IOException e)
    {
        log.error("Exception while trying to create " + TARGET_FILE,e);
    }
}
Collapse
 
florianschaetz profile image
(((Florian Schätz)))

I believe one of the important things to keep in mind when using lambdas and streams is to keep it readable. This can be done by explicitly declaring functions, filters, etc. or to use method references instead of nesting the code in complex statements.

By putting the code into methods, for example, anyone reading the code can start at the easily understandable "top layer", getting the big picture view. With well named methods, they then can check how exactly does, for example, this::replacePlaceholders do its job - but for the big picture, it's just important to know "Ah, and now he's somehow replacing the placeholders".

Unless the statement is very easy, I tend to refactor it into a method reference most of the time (since defining functions in the method itself also makes the method bigger and more complex).