Olaganathan.S
Olaganathan.S

Reputation: 351

Do I have to close FileInputStream?

I am working as a trainee in Test Automation. I am working with creating Junit code with Eclipse and run using Eclipse. In that I am retriving the datas from excel sheet using FileInputStream function.

FileInputStream fi=new FileInputStream("c:\\search.xls");
Workbook w=Workbook.getWorkbook(fi);
Sheet s=w.getSheet(0);

Is it necessary to close the Inputstream function? If it so please guide me with some codings.

Upvotes: 35

Views: 64610

Answers (10)

cquezel
cquezel

Reputation: 4487

The Workbook implements the Closeable interface which indicates that you should call the close() method or use a try with resources to free resources acquired by a Workbook object.

You can’t conclude anything on how the InputStream resource is used by the Workbook by simply observing that the Workbook has a close() method.

What really goes on is that the Workbook constructor uses the stream to initialize itself. The constructor does not keep a reference to the stream. The Workbook constructor does not close the InputStream. It is your responsibility to close the InputStream and you can do this immediately after the Workbook object is constructed. The following code is therefore OK:

private static Workbook getWorkbook() throws IOException {
    try (InputStream is = ...) {
        return WorkbookFactory.create(is); // newer API
    }
}

    try (Workbook workbook = getWorkbook()) {
        Sheet sheet = workbook.getSheet("SheetName");
        ...
    }

Upvotes: 1

pcjuzer
pcjuzer

Reputation: 2824

It's always a good idea to close resources you use, BUT:

If you use resource A in resource B, it's sensible to close B instead of A if it has a method for it.

In your case, you use FileInputStream in Workbook, so you'd better to close Workbook and rely on Workbook that it will close FileInputStream.

In this particular case, actually, Workbook will close FileInputStream at the end of the getWorkbook() method but it's still a good idea to close Workbook to be able to be garbage collected.

Upvotes: 7

Sandip Shinde
Sandip Shinde

Reputation: 1

Do something like this.

FileInputStream fi=null;
try{
fi = new FileInputStream("c:\\search.xls");
Workbook w=Workbook.getWorkbook(fi);
Sheet s=w.getSheet(0);
}catch(IOException ioe){
}finally{
if(fi != null){
fi.close();
}
fi = null;//This will be hint to get finalize() called on fi so that underlying resources used will released like files opened.
}

Upvotes: 0

Yu Jiaao
Yu Jiaao

Reputation: 4714

I do such a way to ensure to close the excel file input stream, this maybe helps

abstract int workWithWorkBook(Workbook workBook);

protected int doWorkBook(Path excelFile) throws IOException {
    File f = excelFile.toFile();

    try (FileInputStream excelContent = new FileInputStream(excelFile.toFile())){
        POIFSFileSystem fileSystem = new POIFSFileSystem(excelContent);
        Workbook workBook = null;
        if (f.getName().endsWith("xls")) {
            workBook = new HSSFWorkbook(fileSystem);
        } else if (f.getName().endsWith("xlsx")) {
            workBook = new XSSFWorkbook(excelContent);
        }
        return workWithWorkBook(workBook);

    }catch (Exception e){
        e.printStackTrace();
        throw e;
    }
}

9b9ea92b-5b63-47f9-a865-fd40dd602cd5

Upvotes: 0

Tony
Tony

Reputation: 151

Recently, when I tried to refactor my code, I had to move the workbook creation to another method, and the FileInputStream is created in that method. That method creates a FileInputStream and returns a Workbook. But FileInputStream is not visible from the main method; so how will I close my FileInputStream at the end of main method? The answer is, you don't have to close FileInputStream, instead you just close the workbook, which internally closes FileInputStream. In short, it is incorrect to say that you must close FileInputStream no matter what.

Upvotes: 0

full_stuck_developer
full_stuck_developer

Reputation: 235

Yes! you should always release the resources once after you are done with them. Java has a powerful mechanism for Garbage Collection(note that it is different thing compare to resource management/leaks.) So a Garbage collector can not determine that if you need the resource in future or not? Failing to release resources may cause issues like- Denial of services, poor performance .

As already answered but another effort less way is try with resources

    try (FileInputStream fi = new FileInputStream("c:\\search.xls")) {

         //do something with fi.
         //fi.getChannel() ;

    } catch(IOException e) {
        // exception handling.
    } finally {
    // some statements for finally.
   }

Now you don't need to explicitly call fi.close() method.

Upvotes: 6

asgs
asgs

Reputation: 3984

Yes, you need to close the inputstream if you want your system resources released back.

FileInputStream.close() is what you need.

Upvotes: 36

luis.espinal
luis.espinal

Reputation: 10519

Basic CompSci 101 tell us to make sure to close resources that we open, in Java or any language. So yes, you need to close them. Bad juju is bound to happen when you do not do so.

Also, you should learn (and have the inclination) to use the Javadocs. Look up at the Javadoc for FileInputStream and Closeable. The answers are there.

Upvotes: -1

Peter Lawrey
Peter Lawrey

Reputation: 533472

You either need to close(), or end your program.

However you can run into confusing issues if you don't close the file as

  • sometimes test are run individually or a group of test are run in the same process. (So you could have a test which works one way but not the other)
  • you cannot rename or delete an open file.

It is best practice to always close your resources which you are finished with them, however I see unit tests as scripts which don't always have to follow best practice.

Upvotes: 10

Axel
Axel

Reputation: 14149

FileInputStream fi=null;
try {
    fi=new FileInputStream("c:\\search.xls");
    Workbook w=Workbook.getWorkbook(fi);
    Sheet s=w.getSheet(0);
} finally {
    if (fi!=null) {
        fi.close();
    }
}

Upvotes: 9

Related Questions