Extract a range of rows from one file into another

Question

Wondering if there is anything I can improve upon my code?

This method accepts a filename (usually will be the path to the file in txt format), the starting row count of that file and the total amount of rows of the file. The method is suppose to iterate through the starting row count passed in and then write into a new file with the remaining rows.

public void blah(String fileName, String startRowCount, String totalRowCount) {
 BufferedReader br = null; 
 File newFile = null;
 BufferedWriter output = null;
 StringBuilder sb = null;
 int startRowCountInt = Integer.parseInt(startRowCount);
 int totalRowCountInt = Integer.parseInt(totalRowCount); 
 try {
 br = new BufferedReader(new FileReader(fileName));
 sb = new StringBuilder();
 newFile = new File("name_Rerun.txt");
 output = new BufferedWriter(new FileWriter(newFile)); 
 String line = "";
 int counter = 0; 
 while ((line = br.readLine()) != null) { 
 if (startRowCountInt <= counter && counter <= totalRowCountInt) { 
 output.write(line);
 output.newLine();
 }
 counter++;
 } 
 } catch (IOException e) {
 // TODO Auto-generated catch block
 LOGGER.info("File was not found.");
 e.printStackTrace();
 } finally {
 // Should update to Java 7 in order to use try with resources and then this whole finally block can be removed. 
 try {
 if ( br != null ) {
 br.close();
 } 
 if ( output != null ) {
 output.close();
 }
 } catch (IOException e) {
 // TODO Auto-generated catch block
 LOGGER.info("Couldn't close BufferReader.");
 e.printStackTrace();
 }
 }
 }

Answer 1

public void blah(String fileName, String startRowCount, String totalRowCount)

A general purpose function should not have integer parameters passed as string, it's not its responsibility to parse them. Change them to int and move parsing outside. Side note: from given code it's not clear but parsing may fail if input is user provided (otherwise I see no reason you may have a string). In that case Integer.parseInt() will throw NumberFormatException.

br, sb but also newFile and counter aren't best variable names you can think of. Not to mention that you do not even use sb. Let's say you have these:

reader = new BufferedReader(new FileReader(inputFileName));
writer = new BufferedWriter(new FileWriter(outputFileName));

Note that I also changed parameter name and I introduced a constant for output file name. I'm generally against paths relative to current working directory but this may be a requirement in this case (I don't know.) If you're using Java 8 you also have an handy Files.lines(Paths.get(inputFileName)). You may also consider to use PrintWriter (which has a println() method) instead of BufferedWriter.

Your if condition inside while checks for two different things: skipping lines until first required one is read and ignore lines after last required one is read. If you do not require to consume all input lines then you're wasting time because you will read entire file even when you're done with your task. Few changes about names, you're counting line indices, not count.

String line = "";
int currentLineNumber = 0; 
while ((line = br.readLine()) != null) { 
 if (counter++ < startRowIndex)
 continue;
 if (counter > lastRowIndex)
 break;
 output.write(line);
 output.newLine();
}

Your pattern to catch IOException is little bit redundant, you do not need to log the inner error for close(), any relevant error might come from a previous call to flush():

try {
 // Do your stuff here...
 writer.flush();
} finally {
 if (writer != null) {
 try {
 writer.close();
 } catch (IOException e) {
 // We may ignore errors for close(), if anything went
 // wrong with our data then we already reported the issue
 // with the call to flush()
 }
 }
}

Note, however, that this pattern is prolix and it may be simplified (as you already marked in a comment) using try-with-resources:

try (BufferedReader reader = new BufferedReader(new FileReader(inputFilePath))) {
 // Do your stuff here...
} catch (IOException e) {
 // Log?
}

• robben: Thanks! I tried the if condition you stated but it gives the wrong output.
• Adriano Repetti: Fixed, inverted condition (I guess). If it's one line off you may want to set initial value to 1