Create Excel file multiple times with different names

By Posted on

Problem

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.

Solution

I would write it completely differently.

import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import static java.nio.file.StandardOpenOption.CREATE_NEW;
import java.text.MessageFormat;
import java.util.Date;

…

/**
 * A MessageFormat for the file path that takes three parameters:
 * 0: username
 * 1: date
 * 2: collision counter
 */
private static final String PATH_FMT = 
    "C:\Users\{0}\Desktop\Bloomberg Rechnung-{1,date,yyyyMMdd}{2,choice,0< ({2})}.xlsx";

private File save() throws IOException {
    Date now = new Date();
    for (int fCounter = 0; ; fCounter++) {
        Path path = Paths.get(
            MessageFormat.format(PATH_FMT, this.username, now, fCounter)
        );
        try (OutputStream out = Files.newOutputStream(path, CREATE_NEW)) {
            this.wb.write(out);
            this.wb.close();
            return path.toFile();
        } catch (FileAlreadyExistsException incrCounterAndRetry) {
        }
    }
}

Observations:

  • This method doesn’t just create an empty file; it also writes the contents of the Excel file. The common terminology is “save”, so the method should be named accordingly.
  • As @Spotted implicitly suggested, the yyyyMMdd format is technically superior, mainly in that it sorts correctly. Use it unless you have a good reason not to.

  • Testing File.exists() is not a very good way to avoid collisions. For one, you could have a race condition in the time gap between the check for existence and the creation of the file. For another, it’s inefficient, in that it requires additional system calls.

    A better solution would be Paths.newOutputStream(path, CREATE_NEW).

  • MessageFormat is a handy way to do complex string interpolation. It can even do date formatting and a switch based on a number.
  • FileNotFoundException is a kind of IOException. You don’t need to declare it redundantly.

  • You’re swallowing a lot of exceptions with catch (Exception e). And if you have debug-level logging suppressed, you would never see what went wrong.

    The logging should at least be at the error level. Better yet, I would just propagate any IOException to the caller.

  • You used a try-with-resources block, which is good. That makes out.close() redundant.
  • You don’t need System.currentTimeMillis(); the default Date() constructor will do.
  • What is filepath? It seems like username would be a better variable name.
  • I think fCounter should be a local variable rather than an instance variable.

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:

  1. Exceptions are (badly) swallowed
  2. 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.
  3. 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:

  1. The hardcoded string for the path
  2. 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 {
    LOG.debug("Create File");
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    File f = Paths.get(folderTo, "Bloomberg Rechnung-" + timestamp + ".xlsx").toFile();
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
    }
    return f;
}

Going even further

As I said in the first chapter, the method has actually 2 responsibilities. I would split those in something like that:

private static File createFile(String folderTo) {
    String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
    return Paths.get(folderTo, "Bloomberg Rechnung-" + timestamp + ".xlsx").toFile();
}

private static void saveWorkbook(XSSFWorkbook wb, File f) throws FileNotFoundException, IOException {
    try (FileOutputStream outputStream = new FileOutputStream(f)) {
        wb.write(outputStream);
    }
}

I also voluntarily removed the log statement. I’m not sure it really add any value when only logging “Create File”. Either add more information such as where the file was created, with which name, etc. or get rid of it.

In the end you could use these methods like this:

saveWorkbook(wb, createFile("C:UsersDesktop"));

or if you need to store a reference on the File:

File f = createFile("C:UsersDesktop");
saveWorkbook(wb, f);

Update

Following your comment, this is how I would implement the file creation based only on a date + a counter (if needed)

private static File createFile(String folderTo) {
    String date = LocalDate.now().format(DateTimeFormatter.ofPattern("yyyyMMdd"));
    return createUniqueFileForDate(folderTo, date);
}

private static File createUniqueFileForDate(String folderTo, String date) {
    String index = "";
    File f = null;
    for (int i = 1;; i++) {
        f = Paths.get(folderTo, "Bloomberg Rechnung-" + date + index + ".xlsx").toFile();
        if (!f.exists()) {
            break;
        }
        index = " (" + i + ")";
    }
    return f;
}

Leave a Reply

Your email address will not be published. Required fields are marked *