Java code optimization, replacing all chars in a file

Question

I have tried doing it like this:

import java.io.*;

public class ConvertChar {
    public static void main(String args[]) {
        Long now = System.nanoTime();
        String nomCompletFichier = "C:\\Users\\aahamed\\Desktop\\test\\test.xml";
        Convert(nomCompletFichier);
        Long inter = System.nanoTime() - now;
        System.out.println(inter);
    }

    public static void Convert(String nomCompletFichier) {
        FileWriter writer = null;
        BufferedReader reader = null;
        try {
            File file = new File(nomCompletFichier);
            reader = new BufferedReader(new FileReader(file));

            String oldtext = "";
            while (reader.ready()) {
                oldtext += reader.readLine() + "\n";
            }
            reader.close();
            // replace a word in a file
            // String newtext = oldtext.replaceAll("drink", "Love");

            // To replace a line in a file
            String newtext = oldtext.replaceAll("&(?!amp;)", "&amp;");

            writer = new FileWriter(file);
            writer.write(newtext);
            writer.close();

        } catch (IOException ioe) {
            ioe.printStackTrace();
        }
    }

}

However the code above takes more time to execute than creating two different files:

import java.io.*;

public class ConvertChar {
    public static void main(String args[]) {
        Long now = System.nanoTime();
        String nomCompletFichier = "C:\\Users\\aahamed\\Desktop\\test\\test.xml";
        Convert(nomCompletFichier);
        Long inter = System.nanoTime() - now;
        System.out.println(inter);
    }

    private static void Convert(String nomCompletFichier) {

        BufferedReader br = null;
        BufferedWriter bw = null;

        try {
            File file = new File(nomCompletFichier);
            File tempFile = File.createTempFile("buffer", ".tmp");

            bw = new BufferedWriter(new FileWriter(tempFile, true));
            br = new BufferedReader(new FileReader(file));

            while (br.ready()) {
                bw.write(br.readLine().replaceAll("&(?!amp;)", "&amp;") + "\n");
            }

            bw.close();
            br.close();

            file.delete();
            tempFile.renameTo(file);

        } catch (IOException e) {
            // writeLog("Erreur lors de la conversion des caractères : " + e.getMessage(), 0);
        } finally {
            try {
                bw.close();
            } catch (Exception ignore) {
            }
            try {
                br.close();
            } catch (Exception ignore) {
            }
        }
    }

}

Is there any way to do the 2nd code without creating a temp file and reducing the execution time? I am doing a code optimization.

In the second example, why don't you write directly to the target file? Why do you need a temporary file? — Mifeet, Apr 28 '16 at 11:54
"do the 2nd code without creating a temp file": you mean using a temp file without creating a temp file? — Henry, Apr 28 '16 at 11:54
@Henry: I mean replacing all the chars in the file without using a temp file at all. Do the change in the file directly. — refresh, Apr 28 '16 at 11:55
@Mifeet: When I write directly in the target file, it doubles everything. I mean it reads a line, replace the values and then write it in the file without deleting the original lines. — refresh, Apr 28 '16 at 11:56

score 5 · Answer 1 · answered Apr 28 '16 at 11:56

The main reason why your first program is slow is probably that it's building up the string oldtext incrementally. The problem with that is that each time you add another line to it it may need to make a copy of it. Since each copy takes time roughly proportional to the length of the string being copied, your execution time will scale like the square of the size of your input file.

You can check whether this is your problem by trying with files of different lengths and seeing how the runtime depends on the file size.

If so, one easy way to get around the problem is Java's StringBuilder class which is intended for exactly this task: building up a large string incrementally.

score 1 · Accepted Answer · edited May 23 '17 at 11:59

The main culprit in your first example is that you're building oldtext inefficiently using String concatenations, as explained here. This allocates a new string for every concatenation. Java provides you StringBuilder for building strings:

    StringBuilder builder = new StringBuilder;
    while(reader.ready()){
        builder.append(reader.readLine());
        builder.append("\n");
    }
    String oldtext = builder.toString();

You can also do the replacement when you're building your text in StringBuilder. Another problem with your code is that you shouldn't use ready() to check if there is some content left in the file - check the result of readLine(). Finally, closing the stream should be in a finally or try-with-resources block. The result could look like this:

    StringBuilder builder = new StringBuilder();
    try (BufferedReader reader = new BufferedReader(new FileReader(file))) {
        String line = reader.readLine();
        while (line != null) {
            builder.append(line.replaceAll("&(?!amp;)", "&amp;"));
            builder.append('\n');
            line = reader.readLine();
        }
    }
    String newText = builder.toString();

Writing to a temporary file is a good solution too, though. The amount of I/O, which is the slowest to handle, is the same in both cases - read the full content once, write result once.

Note: split the `append` into 2 calls to avoid creating a new string for each line when you do the concatenation. — Andy Turner, Apr 28 '16 at 11:58
@AndyTurner: can you please show an example of what you mean? — refresh, Apr 28 '16 at 12:01
@Mifeet a couple of points 1) Don't create resources inline like that with try-with-resources: your `FileReader` won't get closed if there is an exception when creating the `BufferedReader`: create a separate resource variable; 2) `while ((line = reader.readLine()) != null) {` is a bit less repetitive. — Andy Turner, Apr 28 '16 at 12:06

Java code optimization, replacing all chars in a file

2 Answers2