2

Possible Duplicate:
How to create a Java String from the contents of a file

I am using a BufferedReader to read a large text file and want to store the entirety of what the reader reads into a String. This does not seem to be working correctly as when I print out the String, it seems as if it only grabbed the last portion of the file. Am I doing something wrong here?

try {
    String str = "";
    BufferedReader fileReader = new BufferedReader(new FileReader(args[2]));
    while (fileReader.ready()) {
        str += (char) fileReader.read();
    }
    System.out.println(str);
} catch (IOException e) {
    e.printStackTrace();
}
aboger
  • 2,214
  • 6
  • 33
  • 47
ILostMySpoon
  • 2,399
  • 2
  • 19
  • 25

5 Answers5

2

Am I doing something wrong here?

Well, some things wrong, and other things far from ideal.

  • You've got a variable of type BufferedReader called fileReader. That's confusing to say the least.
  • You're using FileReader which is a generally bad idea as it always uses the platform default encoding
  • You're only reading while ready() returns true. That just returns whether or not the next read will block - it may be okay for files, but it's definitely not a good idea in general. You should read until the next call indicates that you've exhausted the stream.
  • You're reading a character at a time, which is somewhat inefficient - there's no need to make one call per character rather than using the overload of read which takes a character array, allowing bulk transfer.
  • You're using string concatenation to build up the file, which is also very inefficient.
  • There's no indication that you're closing the reader. Maybe that's in code which you haven't posted...
  • You've got two levels of try block for no obvious reason, and your IOException handling is almost always the wrong approach - you should very rarely swallow exceptions (even after logging) and then continue as if nothing happened.

If you possibly can, avoid writing this code completely - use Guava instead:

// Use the appropriate encoding for the file, of course.
String text = Files.toString(new File(args[2]), Charsets.UTF_8);

Now of course, you may well find that you still see the same result - maybe the file has "\r" as the line break and you're on a system which only interprets that as "return to start of line" so each line is overwriting the previous one. We can't really tell that - but once you've replaced your code with a single call to Files.toString(), it'll be easier to diagnose that.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    I don't see why reading character by character on a BufferedReader would be so inefficient. Otherwise, +1. – JB Nizet Oct 10 '12 at 21:54
  • @JBNizet: Edited. It's not as bad on `BufferedReader` as elsewhere, but it's still needlessly inefficient. – Jon Skeet Oct 10 '12 at 21:55
  • I just did a micro-benchmark, and it is indeed significantly less efficient. Although I always use a buffer, I wouldn't have thought the difference would be so large (around 30 times faster with a buffer) – JB Nizet Oct 10 '12 at 22:13
  • I am reading in character by character because I am writing this for an assignment. Since I cannot use Guava, are there any pointers you can provide to me in regards to improving what I have written to achieve my goal? Thanks for all the help so far. – ILostMySpoon Oct 10 '12 at 23:21
  • 1
    @user1736256: I don't see why "writing this for an assignment" is any reason to read character by character. I've given you quite a few pointers for improvements already - but another would be to use `StringBuilder` rather than string concatenation. And even if you can't use Guava for your final code, you could at least use it to diagnose this part of the problem. – Jon Skeet Oct 11 '12 at 06:05
2

Your problem is the while condition. You shouldn't use ready there. By the way, please, replace your String with StringBuffer your code will run much faster.

Try with this code (untested but should work)

StringBuffer sb = new StringBuffer();

try {
    fileReader = new BufferedReader(new FileReader(args[2]));
    int i;
    while ((i=fileReader.read())!=-1) {
            sb.append(s);
    }

    System.out.println(sb.toString());

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

Here a version using readLine (if you care about newlines you can still append a \n)

StringBuffer sb = new StringBuffer();

try {
    fileReader = new BufferedReader(new FileReader(args[2]));
    String s;
    while ((s=fileReader.readLine())!=null) {
            sb.append(s);
            //sb.append('\n'); //if you want the newline
    }

    System.out.println(sb.toString());

 } catch (IOException e) {
        e.printStackTrace();
 }
Marco Martinelli
  • 892
  • 2
  • 14
  • 27
1

You can also use one of the IOUtils.toString methods of commons ioutils.

ali köksal
  • 1,217
  • 1
  • 12
  • 19
0

If you're on Java 7, use Files.readAllLines to do this in one line.

Otherwise, your method is quite inefficient. Use one of the answers from here How do I create a Java string from the contents of a file?

Community
  • 1
  • 1
artbristol
  • 32,010
  • 5
  • 70
  • 103
0

If this is a text file, why don't you use readLine()?

MByD
  • 135,866
  • 28
  • 264
  • 277