7

I found this code from another question

private void updateLine(String toUpdate, String updated) throws IOException {
    BufferedReader file = new BufferedReader(new FileReader(data));
    String line;
    String input = "";

    while ((line = file.readLine()) != null)
        input += line + "\n";

    input = input.replace(toUpdate, updated);

    FileOutputStream os = new FileOutputStream(data);
    os.write(input.getBytes());

    file.close();
    os.close();
}

This is my file before I replace some lines

example1
example2
example3

But when I replace a line, the file now looks like this

example1example2example3

Which makes it impossible to read the file when there are a lot of lines in it.

How would I go about editing the code above to make my file look what it looked like at the start?

Robby Cornelissen
  • 91,784
  • 22
  • 134
  • 156
user3879542
  • 125
  • 1
  • 2
  • 11
  • @AstroCB Exactly what I said. My file looks clean, everything there line by line, but when I replace any of the lines, it will mess up and put all of the lines together, as seen in the second example. – user3879542 Aug 09 '14 at 15:57
  • Are you running on a windows based system? So you have to use `\r\n` as line delimiter. – Jens Aug 09 '14 at 15:57
  • @Jens Yes I am running Windows. I've managed to fix this by using System.getProperty("line.separator"), but I'll keep that in mind as well. Thanks. – user3879542 Aug 09 '14 at 16:00
  • 1
    Please use readers , writers and write string inside the loop, dont use + to append strings – Kalpesh Soni Aug 09 '14 at 16:09
  • 1
    @KalpeshSoni This is not my code, I found it on a similar question. I'm not too familiar with this stuff so I don't know what I should edit. – user3879542 Aug 09 '14 at 16:12

3 Answers3

10

Use System.lineSeparator() instead of \n.

while ((line = file.readLine()) != null)
    input += line + System.lineSeparator();

The issue is that on Unix systems, the line separator is \n while on Windows systems, it's \r\n.

In Java versions older then Java 7, you would have to use System.getProperty("line.separator") instead.

As pointed out in the comments, if you have concerns about memory usage, it would be wise to not store the entire output in a variable, but write it out line-by-line in the loop that you're using to process the input.

Robby Cornelissen
  • 91,784
  • 22
  • 134
  • 156
  • I use Java6, but using System.getProperty("line.separator") instead of \n worked perfectly. Thanks. – user3879542 Aug 09 '14 at 15:58
  • Please point out that op's code to read entire file in one string is terrible, he should do the writes in the while loop – Kalpesh Soni Aug 09 '14 at 16:07
  • @KalpeshSoni Why don't you add a comment to the question yourself? And, it's only terrible if OP actually needs to worry about memory usage, which might not be the case. – Robby Cornelissen Aug 09 '14 at 16:09
  • developers will never need to worry about memory usage, their system admins will – Kalpesh Soni Aug 09 '14 at 16:10
  • What is the output of an assignment, such as this (line = file.readLine())? I suppose it is the contents of line, but I didn't know you could do that. Can you confirm? – JohnyTex Nov 25 '16 at 18:21
3

If you read and modify line by line this has the advantage, that you dont need to fit the whole file in memory. Not sure if this is possible in your case, but it is generally a good thing to aim for streaming. In your case this would in addition remove the need for concatenate the string and you don't need to select a line terminator, because you can write each single transformed line with println(). It requires to write to a different file, which is generally a good thing as it is crash safe. You would lose data if you rewrite a file and get aborted.

private void updateLine(String toUpdate, String updated) throws IOException {
    BufferedReader file = new BufferedReader(new FileReader(data));
    PrintWriter writer = new PrintWriter(new File(data+".out"), "UTF-8");
    String line;

    while ((line = file.readLine()) != null)
    {
        line = line.replace(toUpdate, updated);
        writer.println(line);
    }
    file.close();
    if (writer.checkError())
        throw new IOException("cannot write");
    writer.close();
}

In this case, it assumes that you need to do the replace only on complete lines, not multiple lines. I also added an explicit encoding and use a writer, as you have a string to output.

eckes
  • 10,103
  • 1
  • 59
  • 71
  • Sorry had to fix my sample, I was copying to much from the original question: it is supposed to use println(), of course. – eckes Aug 09 '14 at 16:25
  • Writer does not have a method called println, however a PrintWriter does. Using this code gives me a StackOverflowError on line `PrintWriter writer = new PrintWriter(new FileWriter(data + ".out"));` – user3879542 Aug 09 '14 at 16:31
  • Yes it is PrintWriter. Not sure about the StackOverflow. – eckes Aug 09 '14 at 16:37
1

This is because you use OutputStream which is better for handling binary data. Try using PrintWriter and don't add any line terminator at the end of the lines. Example is here

Community
  • 1
  • 1
chronix81
  • 21
  • 5