Create Excel file multiple times with different names

I want to create a new Excel files from an Apache POI object. The filename should contain the standard name, the current date and a counter, if necessary, to prevent collisions. I have a working solution here:

private File createFile() throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    DateFormat df = new SimpleDateFormat("ddMMyyyy");
    Date d = new Date(System.currentTimeMillis());
    while (true) {
        File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx");
        File f2 = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + " ("
                + fcounter + ").xlsx");
        if (!f.exists()) {
            try (FileOutputStream outputStream = new FileOutputStream(
                    "C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx")) {
                wb.write(outputStream);
                outputStream.close();
                wb.close();
                return f;
            } catch (Exception e) {
                LOG.debug(e.toString());
            }
        } else if (!new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + " ("
                + fcounter + ").xlsx").exists()) {
            try (FileOutputStream outputStream = new FileOutputStream("C:\\Users\\" + filepath
                    + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + " (" + fcounter + ").xlsx")) {
                wb.write(outputStream);
                outputStream.close();
                wb.close();
                return f2;
            } catch (Exception e) {
                LOG.debug(e.toString());
            }
        }
        fcounter++;
    }
}

Is there an existing solution or a shorter way to do it? I think my code is very long for such a task.

Short answer

Yes, there are shorter ways to do. Here is how I would save a workbook into a file:

private static File createFile(XSSFWorkbook wb, String folderTo) throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = Paths.get(folderTo, "Bloomberg Rechnung-" + timestamp + ".xlsx").toFile();
    try (FileOutputStream fos = new FileOutputStream(f)) {
        wb.write(fos);
    }
    return f;
}

Note that I'm not totally comfortable with this method because it has 2 responsibilities: creating an excel file and filling it.

Long answer

Now you might wonder how I ended with the code above, so let's begin...

Filename requirement

You stated


Note that if the file is not existing it should be created without counter

This kind of requirement is the kind of which clutters the code with at least one if/else and is in general irrelevant. If you are okay to have a number in the filename (going from 1 to n), then why not having 0 ?

After all, the requirement behind that is that no filename should be erased by another with the same name. So the real problem to resolve here is how to make sure this method won't create 2 files with the same name ?

Since you stated


I want to create a new excel file if my program is run

I assumed using a timestamp should be enough (guaranteeing you one unique filename per second).

Here is createFile() after this refactor:

private File createFile() throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    DateFormat df = new SimpleDateFormat("yyyyMMddhhmmss");
    Date d = new Date(System.currentTimeMillis());
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(
            "C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + df.format(d) + ".xlsx")) {
        wb.write(outputStream);
        outputStream.close();
        wb.close();
    } catch (Exception e) {
        LOG.debug(e.toString());
    }
    return f;
}

Using LocalDateTime

Since Java 8, there's a new API to use when you are using dates. I refactored the code to use this new API instead of the old one.

private File createFile() {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(
            "C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx")) {
        wb.write(outputStream);
        outputStream.close();
        wb.close();
    } catch (Exception e) {
        LOG.debug(e.toString());
    }
    return f;
}

Removing duplication

You are creating the File twice. Instead just reuse the existing you already have.

private File createFile() {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
        outputStream.close();
        wb.close();
    } catch (Exception e) {
        LOG.debug(e.toString());
    }
    return f;
}

Removing smells

In the current code, there are (to my sense) three code smells:

• Exceptions are (badly) swallowed

• wb is closed but not opened in this method. This could be a major flaw because it induces a nasty side-effect. It is a general idiom that the one who opens a resource is the one who closes it.

• outputStream is declared in the try, no need to close it manually ! (it will be done automatically)

Since in the original method, you declared throws FileNotFoundException, IOException, I assumed it's ok to remove the try/catch.

private File createFile() throws FileNotFoundException, IOException {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = new File("C:\\Users\\" + filepath + "\\Desktop\\Bloomberg Rechnung-" + timestamp + ".xlsx");
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
    }
    return f;
}

Going further

So now, I could have stopped here. But something is still bothering me:

• The hardcoded string for the path

• The fact that you directly depends on wb, making the method hard to test

The method could event be marked static so anyone reading this code knows that this method doesn't have a side-effect on an object attribute.

```
private static File createFile(XSSFWorkbook wb, String folderTo) throws FileNotFoundException, IOException {

StackExchange Code Review Q#148728, answer score: 2