java after reading text from file, prints will "null" in the beginning

Question

I've readed from the .txt file this:

<?xml version="1.0"?>
<!DOCTYPE cross-domain-policy SYSTEM "/xml/dtds/cross-domain-policy.dtd">
<cross-domain-policy>
   <allow-access-from domain="*" to-ports="*" />
</cross-domain-policy>

but after reading it and then outputing I get this:

null<?xml version="1.0"?>
<!DOCTYPE cross-domain-policy SYSTEM "/xml/dtds/cross-domain-policy.dtd">
<cross-domain-policy>
   <allow-access-from domain="*" to-ports="*" />
</cross-domain-policy>

null at the beginning, this is the method:

public class Filer {

    private static String str;
    public static String read(String file) {

        BufferedReader br = null;
        try {

            String sCurrentLine;

            br = new BufferedReader(new FileReader(file));

            while ((sCurrentLine = br.readLine()) != null) {

                str += sCurrentLine;
            }

        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            try {
                if (br != null)br.close();
            } catch (IOException ex) {
                ex.printStackTrace();
            }
        }

        return str;

    }
}

the returned string its being outputed by System.out.println

I've tried to check that string by str.indexOf("null"); or str.indexOf("\0"); but I get -1

any ideas on how to fix this?

Answer 1

Consider the very first iteration of your loop. The value of str is null, and the value of sCurrentLine is "<?xml version="1.0"?>".

In string concatenation, a null reference is converted into a string of "null". For example:

String x = null;
String y = "a";
String z = x + y; // "nulla"

So on your first iteration, when you execute:

str += sCurrentLine;

the value of str will be "null<?xml version="1.0"?>"

You could just initialize str to ""... but I wouldn't.

There's no reason why it should be a static variable anyway - would you really want to keep the old value hanging around even if the method is called again? That would be very odd. It should be a local variable.

Also, you shouldn't use string concatenation in a loop like this - it's horrible in terms of performance, because it needs to keep copying the old data. Use a StringBuilder instead.

I wouldn't catch exceptions like that either - do you really want the caller not to know that anything went wrong? Just declare that your method can throw IOException.

Additionally, I wouldn't use FileReader myself - it gives you no control over the encoding used to read the file - it will always use the platform default encoding, which may be inappropriate. I'd use a FileInputStream wrapped in an InputStreamReader. You can then use a try-with-resources statement in Java 7 to close the FileInputStream automatically.

So, with all those changes, your code would become something like this:

public static String read(String file) throws IOException {
    StringBuilder builder = new StringBuilder();
    try (InputStream input = new FileInputStream(file)) {
        BufferedReader reader = new BufferedReader(
            new InputStreamReader(input, "UTF-8"));
        String line;
        while ((line = reader.readLine()) != null) {
            builder.append(line);
        }
    }
    return builder.toString();
}

Note that this will convert a multi-line file into a single line of text - use builder.append("\n") or something similar in the loop if you want to preserve the number of lines (but don't care about line separators). If you want to preserve the exact line endings, don't use readLine - just read characters into a buffer instead.

Also, consider using a third party library such as Guava to do all this instead - for example, you could use Files.toString().