Haripriya
Haripriya

Reputation: 1000

Sonarqube issue - Change this "try" to a try-with-resources. How to handle conditional resources?

Checked similar questions regarding this topic but I did not find solution to my use-case.
Following method shows Sonar issue with Major code smell as - Change this "try" to a try-with-resources.

private void readExcel() {
        Workbook workbook = null;
        BufferedReader br = null;
        
        try {
            File file = path.toFile();
            if (file.getName().endsWith(FILE_TYPES.XLS.value) && (file.length() / (1024 * 1024)) < 25) {
                workbook = new HSSFWorkbook(new POIFSFileSystem(file));
                Sheet sheet = workbook.getSheetAt(0);
                readExcel(sheet);
            } else if ((file.getName().endsWith(FILE_TYPES.XLSX.value)) && (file.length() / (1024 * 1024)) < 25) {
                workbook = new XSSFWorkbook(file);
                Sheet sheet = workbook.getSheetAt(0);
                readExcel(sheet);
            } else if (file.getName().endsWith(FILE_TYPES.CSV.value)) {
                // Apache POI cant read/write CSV. Hence using Java IO.
                br = new BufferedReader(new FileReader(file));
                readExcel(br);
            }
        } catch (IOException | InvalidFormatException ex) {
                // set invalid flags and call clean-up
        } finally {
            try {
                if (workbook != null) {
                    workbook.close();
                }
                if (br != null) {
                    br.close();
                }
            } catch (IOException ex) {
                // set invalid flags and call clean-up
            }
        } // try-catch-finally closed
    }

Is this false positive sonar issue?

Upvotes: 2

Views: 8756

Answers (1)

Andreas
Andreas

Reputation: 159114

HSSFWorkbook is an AutoCloseable.
XSSFWorkbook is an AutoCloseable.
BufferedReader is an AutoCloseable.

They all need their own try-with-resources.

Get rid of Workbook workbook = null; and BufferedReader br = null;, and the code in the finally block, because that's all old-style pre-try-with-resources.

private void readExcel() {
    try {
        File file = path.toFile();
        if (file.getName().endsWith(FILE_TYPES.XLS.value) && (file.length() / (1024 * 1024)) < 25) {
            try (Workbook workbook = new HSSFWorkbook(new POIFSFileSystem(file))) {
                Sheet sheet = workbook.getSheetAt(0);
                readExcel(sheet);
            }
        } else if ((file.getName().endsWith(FILE_TYPES.XLSX.value)) && (file.length() / (1024 * 1024)) < 25) {
            try (Workbook workbook = new XSSFWorkbook(file)) {
                Sheet sheet = workbook.getSheetAt(0);
                readExcel(sheet);
            }
        } else if (file.getName().endsWith(FILE_TYPES.CSV.value)) {
            // Apache POI cant read/write CSV. Hence using Java IO.
            try (BufferedReader br = new BufferedReader(new FileReader(file))) {
                readExcel(br);
            }
        }
    } catch (IOException | InvalidFormatException ex) {
        // set invalid flags and call clean-up
    }
}

Upvotes: 4

Related Questions