0

Can someone help me downport this to Java 7? And can you explain how you did it?

Stream.of(arenaDirectory.list()).filter((file) ->
        Stream.of(new File(arenaDirectory, file).list()).filter((childName) -> childName.equals(DATA_FILE_NAME)).count() > 0
    ).map((file) -> new File(arenaDirectory, file)).forEach((arenaFolder) -> {
}

The arena directory is a folder.

5
  • Can we assume that arenaDirectory is a java.util.File? Commented Apr 14, 2016 at 4:01
  • @MadProgrammer What's a java.util.File? Commented Apr 14, 2016 at 4:10
  • It looks like this code goes through all the subdirectories of a directory, selects those which meet a certain condition, goes through all the files in the selected subdirectories, and then does absolutely nothing with them. So I think the easiest way to downport is to replace the above code with ; Commented Apr 14, 2016 at 4:18
  • Just kidding, I'm sure the intent is to put some other code inside the last pair of curly braces Commented Apr 14, 2016 at 4:19
  • 1
    @ajb Brain on backwards, java.io.File Commented Apr 14, 2016 at 4:30

3 Answers 3

1

The easiest way to get rid of a Stream is to use an enhanced for loop. So when you see Stream.of on an array A, you can replace it with for (Type x : A). So start with

for (String file : arenaDirectory.list()) {
}

The operations on the Stream will then be what goes in the loop. The first is filter, which will eliminate elements of the stream and keep only those that meet a condition. That's equivalent to an if statement:

for (String file : arenaDirectory.list()) {
     if (Stream.of(new File(arenaDirectory, file).list()).filter((childName) -> childName.equals(DATA_FILE_NAME)).count() > 0) {
     }
}

Note that since I chose the same name for the loop variable as the original code used in the lambda, it's easy to just copy the predicate part of the lambda expression into the if.

Now you have map, which operates on the elements that the filter didn't eliminate and transforms them to something new. So you'll put the operation inside the if, and declare a variable to hold the new value:

for (String file : arenaDirectory.list()) {
     if (Stream.of(new File(arenaDirectory, file).list()).filter((childName) -> childName.equals(DATA_FILE_NAME)).count() > 0) {
         File arenaFolder = new File(arenaDirectory, file);
     }
}

Again, I chose the same variable name that the code used in the next lambda.

The code then has forEach, which means to perform an operation on the new values returned by the map--which, in this case, is the arenaFolder variable created above. Here, I don't know what the code is, since your post had empty curly braces. But whatever it is, you will put it after the last statement; it will use arenaFolder for something:

for (String file : arenaDirectory.list()) {
     if (Stream.of(new File(arenaDirectory, file).list()).filter((childName) -> childName.equals(DATA_FILE_NAME)).count() > 0) {
         File arenaFolder = new File(arenaDirectory, file);
         // code that does something with arenaFolder
     }
}

Now all you have to do is follow the same steps to downport the nested stream. Instead of the count() operation, you'll have to keep your own counter--go through the loop and increment it whenever the "filter" condition is true. Or, since you're just using count to see if any condition matches, you don't really need a counter--just break as soon as you find a match.

Sign up to request clarification or add additional context in comments.

7 Comments

Moving the nested Stream (the one inside a filter) to a well-named helper method may help clarify code.
Right. The original code was scary. The nested stream should have been in a helper method in the original code, and it should be moved to a helper in the Java 7 code also.
Yeah, the code could also have been simplified by replacing list() with listFiles(), since the only use of file was two instances of new File(arenaDirectory, file), which you'll get for free using listFiles().
Also, wouldn't count() > 0 be better written as findAny().isPresent(), so it becomes short-circuiting? I believe that is your suggestion for the Java 7 version, right? Short-circuiting using break, I mean.
What about the Lambda inside the filter? Isn't -> lambda?
|
1

First, lets clean up the Java8 version, then it will be clearer how to refactor it into loops:

  1. Your code doesn't compile, it needs ); at the end of it.
  2. The formatting makes it really hard to decipher. I'm also going to put it inside of a method for refactoring purposes:

    public void doSomething() {
        Stream.of(arenaDirectory.list())
            .filter((file) ->
                Stream.of(new File(arenaDirectory, file).list())
                    .filter((childName) -> childName.equals(DATA_FILE_NAME))
                    .count() > 0)
            .map((file) -> new File(arenaDirectory, file))
            .forEach((arenaFolder) -> {});
    }
    
  3. Its confusing that we don't do anything with the result, so lets pipe it to System.out.println:

    public void doSomething() {
        Stream.of(arenaDirectory.list())
            .filter((file) ->
                Stream.of(new File(arenaDirectory, file).list())
                    .filter((childName) -> childName.equals(DATA_FILE_NAME))
                    .count() > 0)
            .map((file) -> new File(arenaDirectory, file))
            .forEach(System.out::println);
    }
    
  4. Checking if at-least-one item in a stream matches a predicate is better performed with anyMatch:

    public void doSomething() {
        Stream.of(arenaDirectory.list())
            .filter((file) ->
                Stream.of(new File(arenaDirectory, file).list())
                    .anyMatch((childName) -> childName.equals(DATA_FILE_NAME)))
            .map((file) -> new File(arenaDirectory, file))
            .forEach(System.out::println);
    }
    
  5. Nesting lambdas within lambdas is hard to read, lets factor out the inner lambda to a different method:

    public void doSomething() {
        Stream.of(arenaDirectory.list())
            .filter((file) -> somePredicate(arenaDirectory, file, DATA_FILE_NAME))
            .map((file) -> new File(arenaDirectory, file))
            .forEach(System.out::println);
    }
    
    private boolean somePredicate(File parent, String child, String expectedName) {
        File newFile = new File(parent, child);
        return Stream.of(newFile.list())
            .anyMatch(newChild -> newChild.equals(expectedName));
    }
    
  6. That makes the primary method much more readable! Lets continue with readability improvements... The somePredicate method is confusing because it checks the children of a File it constructs, not the File it was given. Lets clean that up. The primary method gets worse, but we'll clean that up later:

    public void doSomething() {
        Stream.of(arenaDirectory.list())
        .filter((file) -> {
            File dir = new File(arenaDirectory, file);
            return directoryContainsChildNamed(dir, DATA_FILE_NAME);
        })
        .map((file) -> new File(arenaDirectory, file))
        .forEach(System.out::println);
    }    
    private boolean directoryContainsChildNamed(File dir, String expectedName) {
        return Stream.of(dir.list())
            .anyMatch(child -> child.equals(expectedName));
    }
    
  7. Now we see how redundant it is to create the same file twice, so lets clean that up:

    public void doSomething() {
        Stream.of(arenaDirectory.list())
            .map((file) -> new File(arenaDirectory, file))
            .filter((dir) -> directoryContainsChildNamed(dir, DATA_FILE_NAME))
            .forEach(System.out::println);
    }
    
    private boolean directoryContainsChildNamed(File dir, String expectedName) {
        return Stream.of(dir.list())
            .anyMatch(child -> child.equals(expectedName));
    }
    
  8. Really, we don't need the mapping step at all, because there is already an API that returns children as a File[]:

    public void doSomething() {
        Stream.of(arenaDirectory.listFiles())
            .filter((file) -> directoryContainsChildNamed(file, DATA_FILE_NAME))
            .forEach(System.out::println);
    }
    
    private boolean directoryContainsChildNamed(File dir, String expectedName) {
        return Stream.of(dir.list())
            .anyMatch(child -> child.equals(expectedName));
    }
    

OK, now we've gotten somewhere! This is nice clean Java 8, but you wanted Java 7. Since we took the time to clean it up into two simple methods, the conversion is also fairly simple. We need to replace the Streams with loops, and the rest pretty much falls in place:

public void doSomething() {
    for (File file : arenaDirectory.listFiles()) {
        if (directoryContainsChildNamed7(file, DATA_FILE_NAME)) {
            System.out.println(file); // whatever you wanted to do...
        }
    }
}

private boolean directoryContainsChildNamed(File dir, String expectedName) {
    for (String child : dir.list()) {
        if (child.equals(expectedName)) return true;
    }
    return false;
}

Comments

0

In fact it is a too complicated, convoluted form of doing:

Path f = Paths.get(arenaDirectory.getPath(), file, DATA_FILE_NAME);
if (Files.exists(f)) {
    ...
}

I can only guess that maybe the intention was to scan also all subdirectories, which File.list() does not do. Since java 7 (with Path and Files) one can easily walk a directory tree.

Explaining the code seems irrelevant, see the other answers.

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.