3

Description

My goal is to store all file paths in a file. To do that I built a recursive method which browse a specific folder. However, the result is still empty ...

Code

public void main(String[] args) {
    String files = browseFolder("", new File("blablalbla/.../toto"));

    FileWriter writer = new FileWriter(new File("result/"));
    writer.write(files);
    writer.close();
}

private static String browseFolder(String result, File folder) {
    for (final File fileEntry : folder.listFiles()) {
        if (fileEntry.isDirectory()) {
            browseFolder(result, fileEntry);
        }
        else {
            System.out.println(fileEntry.getPath());
            result += fileEntry.getPath() + "\n";
        }
    }
    return result;
}

All files are displayed in the console thanks to the sysout. However, my result file is empty.

Any idea ? Thank you.

Royce
  • 1,557
  • 5
  • 19
  • 44
  • 2
    It's considered a bad idea to assign values to a parameter within the method body. For this problem, you should probably read the answers to [Is Java “pass-by-reference” or “pass-by-value”?](https://stackoverflow.com/questions/40480/is-java-pass-by-reference-or-pass-by-value) – ernest_k Jan 20 '20 at 09:49
  • @ernest_k Thank you. Do you think that is the reason why I'm facing this error : `Exception in thread "main" java.lang.OutOfMemoryError: Java heap space` – Royce Jan 20 '20 at 09:54
  • 1
    Nope. I *guess* you could get that error if you called your method with `"/"` (or `"C:\"`) as `folder` :) – ernest_k Jan 20 '20 at 09:55

3 Answers3

3

Due to immutability of String (variable result always references the same string value in memory), you should use concat to save the result, as it actually creates a new reference to a new string value.

if (fileEntry.isDirectory()) {
    result += browseFolder(result, fileEntry);
}
Steyrix
  • 2,796
  • 1
  • 9
  • 23
2

In

browseFolder(result, fileEntry);

you are ignoring the String returned by the recursive method, which is why you get an empty String in the end.

Instead of concatenating Strings, which results in many String instances being created, I suggest you pass a StringBuilder to your recursive method.

This way your method can modify the StringBuilder, and doesn't have to return a value.

public void main(String[] args) {
    StringBuilder files = new StringBuilder();
    browseFolder(files, new File("blablalbla/.../toto"));

    FileWriter writer = new FileWriter(new File("result/"));
    writer.write(files.toString());
    writer.close();
}

private static void browseFolder(StringBuilder result, File folder) {
    for (final File fileEntry : folder.listFiles()) {
        if (fileEntry.isDirectory()) {
            browseFolder(result, fileEntry);
        } else {
            System.out.println(fileEntry.getPath());
            result.append(fileEntry.getPath()).append("\n");
        }
    }
}
Eran
  • 387,369
  • 54
  • 702
  • 768
2

Note that the recursive call to browseFolder will not change its parameter, because String is immutable. A string variable that is passed to a method will always stay the same after that method returns. This means that as long as the blablalbla/.../toto directory is full of subdirectories, your main call to browseFolder will return an empty string, because you are not doing anything to result.

You could append the return value of recursive browseFolder calls to result, like this:

private static String browseFolder(String result, File folder) {
    for (final File fileEntry : folder.listFiles()) {
        if (fileEntry.isDirectory()) {
            // here:
            result += browseFolder(result, fileEntry);
        }
        else {
            System.out.println(fileEntry.getPath());
            result += fileEntry.getPath() + "\n";
        }
    }
    return result;
}

However, this creates a new string every time you do this, so it is quite wasteful. You can use a StringBuilder (which is mutable!) instead.

private static StringBuilder browseFolder(StringBuilder result, File folder) {
    for (final File fileEntry : folder.listFiles()) {
        if (fileEntry.isDirectory()) {
            browseFolder(result, fileEntry);
        }
        else {
            System.out.println(fileEntry.getPath());
            result.append(fileEntry.getPath()).append("\n");
        }
    }
    return result;
}

The caller will have to change too. You can do .toString on a StringBuilder to convert it to String.

String files = browseFolder(new StringBuilder(), new File("blablalbla/.../toto")).toString();
Sweeper
  • 213,210
  • 22
  • 193
  • 313