patternjavaMinor
Copying contents of two files into one file/ bufferedReader close calls
Viewed 0 times
callsclosefilebufferedreaderintocontentstwofilescopyingone
Problem
I am using the following code to copy the contents of two files into one. Earlier I was not closing the Buff and as eclipse gave warnings I did close it. I want to know if this code is susceptible to failure.
Should I check if br is null in the finally block - my intuition tells me no, because finally will be only executed when try is. I have something over Here which is contradictory to my belief in declaring the variables before try block. What is the best way to do this?
public void mergeFiles() throws IOException{
ArrayList list = new ArrayList();
BufferedReader br = null;
BufferedReader r = null;
try{
br = new BufferedReader(new FileReader(getApplicationContext().getFilesDir()+"/BirthdayReminders/fileone.txt"));
r = new BufferedReader(new FileReader(getApplicationContext().getFilesDir()+"/BirthdayReminders/filetwo.txt"));
String s1 =null;
String s2 = null;
while ((s1 = br.readLine()) != null){
list.add(s1);
}
while((s2 = r.readLine()) != null){
list.add(s2);
}
}
catch (IOException e){
e.printStackTrace();
FLAG =1;
}finally{
br.close();
r.close();
}
BufferedWriter writer=null;
writer = new BufferedWriter(new FileWriter(getApplicationContext().getFilesDir()+"/BirthdayReminders/output.txt"));
String listWord;
for (int i = 0; i< list.size(); i++){
listWord = list.get(i);
writer.write(listWord);
writer.write("\n");
}
writer.close();
}Should I check if br is null in the finally block - my intuition tells me no, because finally will be only executed when try is. I have something over Here which is contradictory to my belief in declaring the variables before try block. What is the best way to do this?
Solution
-
A little bit about code style: it is conventional to surround binary operators with whitespaces(for instance,
should be
-
Yes, you should check that
-
Variable names:
-
This kind of initialization is pointless in my opinion:
This one looks better:
-
The
For-each loop is shorter and simpler.
A little bit about code style: it is conventional to surround binary operators with whitespaces(for instance,
getApplicationContext().getFilesDir()+"/BirthdayReminders/fileone.txt" should be
getApplicationContext().getFilesDir() + "/BirthdayReminders/fileone.txt") and there should be a whitespace after a closing bracket and before the opening one(that is, } finally { is better than }finally{ and so on).-
Yes, you should check that
br and r are not null before calling the close method on them. The FileReader constructor can throw an exception. In this case they will be null. However, you can use try-with-resources(if you have Java 7 or later) to make your code more simple and concise:try (
BufferedReader reader1 = new BufferedReader(new FileReader(...));
BufferedReader reader2 = new BufferedReader(new FileReader(...))
) {
// do something
} catch (IOException e) {
// do something else
}-
Variable names:
r and br are not really descriptive(however, their scope is very narrow so it is not a very big deal in this case). The same holds true for other variables: I would use something like lines instead of list and so on.-
This kind of initialization is pointless in my opinion:
BufferedWriter writer = null;
writer = new BufferedWriter(new FileWriter(getApplicationContext().getFilesDir() + "/BirthdayReminders/output.txt"));This one looks better:
BufferedWriter writer = new BufferedWriter(new FileWriter(getApplicationContext().getFilesDir() + "/BirthdayReminders/output.txt"));-
The
listWord variable and an index-based loop is redundant:for (String listWord: list) {
// do something
}For-each loop is shorter and simpler.
Code Snippets
try (
BufferedReader reader1 = new BufferedReader(new FileReader(...));
BufferedReader reader2 = new BufferedReader(new FileReader(...))
) {
// do something
} catch (IOException e) {
// do something else
}BufferedWriter writer = null;
writer = new BufferedWriter(new FileWriter(getApplicationContext().getFilesDir() + "/BirthdayReminders/output.txt"));BufferedWriter writer = new BufferedWriter(new FileWriter(getApplicationContext().getFilesDir() + "/BirthdayReminders/output.txt"));for (String listWord: list) {
// do something
}Context
StackExchange Code Review Q#77583, answer score: 5
Revisions (0)
No revisions yet.