HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Copying contents of two files into one file/ bufferedReader close calls

Submitted by: @import:stackexchange-codereview··
0
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.

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, 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.