3

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?

Improve this question

4 Answers 4

Reset to default
5

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().

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

1 Comment

chosed this answer as accepted for the full builder example, thank you again
5

When you concatenate a null reference with a string, it is converted to "null" string, and then concatenation is performed.

This is specified in JLS - Section 5.1.11:

If the reference is null, it is converted to the string "null" (four ASCII characters n, u, l, l).

Now, when you declare an instance or static reference field, without initializing to any value, it will be initialized to default value - null, as in your declaration:

private static String str;  // This is `null`.

Then for the first iteration of the loop:

str += sCurrentLine;

is equivalent to:

str = null + sCurrentLine;  // Hence the result.

As for your str.indexOf() query, I don't know where you are testing the index, because it should return the index of null that is 0, provided, you used that method after the loop, as shown in the below snippet:

String str = null;
String str2 = str + "someString";
System.out.println("Index of null in : " + str2 + " : " + str2.indexOf("null"));

This will output to you:

Index of null in : nullsomeString : 0

Also, while performing string concatenation inside such a loop, which you can't control, when it ends, and even in general, you should use StringBuilder instance, to avoid creating intermediate string instances:

StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append(sCurrentLine);
Improve this answer

1 Comment

also why the indexOf didn't find the null that was concatinated?, it returned me -1
1 
private static String str;

is null. You have to initialize it.

private static String str = "";
Improve this answer

Comments

1

do this private static String str = "";. By default, your str variable is null so while concatenating null is used.

Initialize it to a blank string as I've shown.

Improve this answer

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.