Reading and printing a text file in Java

Question

I am trying to read lines from a text file in Java, but there is a lot going on. Is this the best way to do this? I am especially concerned with resource leaks.

import java.io.*;
public class Filing {
 public static void main(String[] args) throws FileNotFoundException, IOException {
 FileReader fr = new FileReader("/Users/thomaspovinelli/Desktop/apps.txt");
 BufferedReader br = new BufferedReader(fr);
 String buffer;
 String fulltext="";
 while ((buffer = br.readLine()) != null) {
 System.out.println(buffer);
 fulltext += buffer;
 }
 br.close();
 fr.close();
 }
}

Answer 1

import java.io.*;

Never import the full package. It is considered bad practice to do so. Instead, import what you need and only what you need.

Instead of just reading a file then printing it, why don't you create a method that reads all line of a file and returns a list of all the lines? This way, you can do more with the result:

public static List<String> getAllLines(File file) throws FileNotFoundException, IOException {
 FileReader fr = new FileReader(file);
 BufferedReader br = new BufferedReader(fr);
 String buffer;
 String fulltext="";
 while ((buffer = br.readLine()) != null) {
 System.out.println(buffer);
 fulltext += buffer;
 }
 br.close();
 fr.close();
}

I am especially concerned with resource leaks.

Since Java 7, there is such thing as a try-with-resources statement, which automatically closes the reader when an exception occurs, which you are not doing here. Your current code has memory leaks!

With some other improvements:

public static List<String> readAllLines(File file) {
 List<String> result = new LinkedList<>();
 try (BufferedReader reader = new BufferedReader(new FileReader(file))) {
 for (String current = reader.readLine(); current != null; current = reader
 .readLine()) {
 result.add(current);
 }
 } catch (IOException e) {
 e.printStackTrace();
 }
 return result;
}

By the way, this is actually the code I use often to read all lines of a file.

• 200_success: It would be safer to construct BufferedReader and FileReader separately rather than as a chain, in case the FileReader constructor succeeds but the BufferedReader constructor fails.
• 200_success: If you want a public static List<String> readAllLines(...), just use java.nio.file.Files.readAllLines(file.toPath()). And I don't see any reason to catch an exception that you don't really know how to handle at that point.

Answer 2

Not sure about the best way, but I'd personally favor one of the options using java.nio and java.util.stream:

Option 1:

public static List<String> readAllLines(String fileName) {
 List<String> result = new ArrayList<>();
 try {
 result.addAll(Files.readAllLines(Paths.get(fileName)));
 } catch (IOException e) {
 e.printStackTrace();
 }
 return result;
}

Option 2:

public static List<String> readAllLines(String fileName) {
 List<String> result = new ArrayList<>();
 try (BufferedReader br = Files.newBufferedReader(Paths.get(fileName))) {
 br.lines().forEachOrdered(result::add);
 } catch (IOException e) {
 e.printStackTrace();
 }
 return result;
}

And if for some reason you cannot use either of those, you might want to have a look at FileUtils#readLines(java.io.File) from Apache Commons IO (although it's probably not worth adding that dependency only for this task).

In general, if you are concerned about (potential) resource leaks and you are using Eclipse, I advise to go to Window > Preferences > Java > Compiler > Errors/Warnings and set the severity level of "Resource leak" and "Potential resource leak" to at least "Warning". This way your IDE will tell you where you need to be careful.

• h.j.k.: I think using ArrayList in lieu of LinkedList should be good enough. :)
• Marvin: @h.j.k.: True, changed it.

Answer 3

FileNotFoundException is a kind of IOException, so you only need to declare throws IOException.

A cleaner way of ensuring that your resources get closed is to use try-with-resources. Write it as fr = ... and br = ... instead of br = new BufferedReader(new FileReader(...)) to ensure that each resource gets closed, even if the FileReader constructor succeeds but the BufferedReader constructor fails.

try (FileReader fr = new FileReader(...);
 BufferedReader br = new BufferedReader(fr)) {
 StringBuilder fullText = new StringBuilder();
 for (String line; (line = br.readLine()) != null; fullText.append(line)) {
 System.out.println(line);
 }
}

Repeated string concatenation (fulltext += buffer) is O(n²): each time you end up re-copying all of fulltext all over again. Use a StringBuilder instead. But in this program, you don't need to, since you never even use fulltext. (By the way, your fulltext is probably not what you want: it has all of the line terminators stripped out, since that's what BufferedReader.readLine() does.)

There's nothing really line-oriented about your program, so there is little reason to scan for line breaks. The simplest way to achieve the same results is using java.nio.file.*:

Files.copy(Paths.get(...), System.out);