Create and back up files

Question

I have to develop a class that creates new files. If a file with the given name already exists, it should be renamed by addition of the timestamp to the file name. The file name should be human-readable, so I can use neither UUID nor createTempFile. The code works, but looks too bulky.

public class FileBackup {

    /**
     * Creates new file. If file with given name exists, then backups
     * it by renaming and adding time stamp to file name;
     *  
     * @param filePath the fully qualified name of file

     * @return the file on success; otherwise returns null
     * 
     * @throws IOException 
     */
    public static File backupAndCreateFile(String filePath) throws IOException  {

        File fileToCreate = new File(filePath);

        boolean backupRequired =  fileToCreate.isFile();

        if (backupRequired && !backupFile(fileToCreate)) {
            return null;
        }

        File file = new File(filePath);
        return file.createNewFile() ? file : null;
    }

    private static final String pattern = "-dd_M_yyyy-hh_mm_ss";
    private static final DateFormat dateFormat = new SimpleDateFormat(pattern);     
    private static String generateBackupName (String filePath) {        
        String name = getFileName (filePath);
        String ext = getFileExtention (filePath);
        return name + dateFormat.format(new Date()) + ext;
    }

    private static String getFileName (String filePath) {   
        int i = filePath.lastIndexOf('.');
        return i == -1 ? filePath : filePath.substring(0, i);
    }

    private static String getFileExtention (String filePath) {  
        int i = filePath.lastIndexOf('.');
        return i == -1 ? "" : filePath.substring(i);
    }

    private static boolean backupFile (File file) {

        String filePath = file.getAbsolutePath();

        while (true) {

            String backupName = generateBackupName (filePath);
            File backup = new File(backupName); 

            if (!backup.isFile()) return file.renameTo(backup);

            // File with generated name exists, so 
            // sleep for 0.5 second and try another name
            try {
                Thread.sleep(500);
            } catch (InterruptedException e) {return false;}; // never should be here       
        }
    }
}

Answer 1

In my opinion your code it's not so bulky but I would do something like below with few changes. I have not tested it too much but it should works.

I will use FilenameUtils for easy manipulation with files like getting basename or extension of file - it will save you some of the lines and also it is one of the most used library for file manipulation.

I would remove String getFileName and String getFileExtention and create self-described function and delete in code comments. I also removed return false for InterruptedException - I would like to get another draft of the name in the next iteration.

I dont like spaggetti code when some string is creating like (with +):

return name + dateFormat.format(new Date()) + ext;

so here I would use StringBuilder or String.format or something like that.

Below is what I have done in few minutes.

import org.apache.commons.io.FilenameUtils;

import java.io.File;
import java.io.IOException;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;

public class FileBackup {

    private static final String pattern = "-dd_M_yyyy-hh_mm_ss.";
    private static final DateFormat dateFormat = new SimpleDateFormat(pattern);

    /**
     * Creates new file. If file with given name exists, then backups
     * it by renaming and adding time stamp to file name;
     *
     * @param filePath the fully qualified name of file

     * @return the file on success; otherwise returns null
     *
     * @throws IOException
     */
    public static File backupAndCreateFile(String filePath) throws IOException {
        return failedBackuOfExistingFile(filePath) ? null : new File(filePath);
    }

    private static boolean failedBackuOfExistingFile(String filePath) {
        File fileOnPath = new File(filePath);
        return  fileOnPath.exists() && !successBackupFile(fileOnPath);
    }

    private static boolean successBackupFile(File fileToBackup) {

        while (true) {

            File backupDraft = new File(generateBackupName(fileToBackup.getAbsolutePath()));
            if (!backupDraft.exists()) {
                return fileToBackup.renameTo(backupDraft);
            }
            fileWithGeneratedNameExistsNowSleepForSomeTime();
        }
    }

    private static void fileWithGeneratedNameExistsNowSleepForSomeTime(){
        try {
            Thread.sleep(500);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }

    private static String generateBackupName (String filePath) {
        String fileName = FilenameUtils.getBaseName(filePath);
        String fileExtension = FilenameUtils.getExtension(filePath);
        return new StringBuilder().append(fileName)
                .append(dateFormat.format(new Date()))
                .append(fileExtension)
                .toString();
    }
}

Answer 2

public static File backupAndCreateFile(String filePath) throws IOException  {
  File fileToCreate = new File(filePath);
  boolean backupRequired =  fileToCreate.isFile();
  if (backupRequired && !backupFile(fileToCreate)) {
    return null;
  }
  File file = new File(filePath);
  return file.createNewFile() ? file : null;
}

• never return NULL unless it is a valid value of the result set. In this case I'd argue. You should throw an (Runtime-)Exception where you return the NULLs.

• There is no need to create a second File object, just return the first one.

• do not rely on side efects in if conditions. Make it explicit, that the backup method will be called only if needed.

• isFile() seems to me a bit suspicious, what if the given name points to a directory?

  public static File backupAndCreateFile(String filePath) throws IOException  {
      File fileToCreate = new File(filePath);
      boolean backupRequired =  fileToCreate.isFile();
      if (fileToCreate.exists())
          if(fileToCreate.isFile()) {
              if(!backupFile(fileToCreate)) 
                  throw new IoException("could not create new backup for file "+fileToCreate);
          } else {
              throw new IoException( fileToCreate+" is not a regular file and cannot be backuped");
          }
      if(  fileToCreate.createNewFile())
          return  fileToCreate;
      throw new IoException("could not create new file "+fileToCreate);
  }
  

Answer 3

Directories

File's can also be directories. You probably want to check this.

Thread.sleep()

Why don't use Random to do this faster?

important-25_10_2016-21_03_55-1526373.txt

isn't less readable than

important-25_10_2016-21_03_55.txt

.

getFileName and getFileExtension

They both use lastIndexOf and so pretty much the same. So why don't bring those two little methods together with theor only use.

Random random = new Random();
private static String generateBackupName(String fileName){
  int period = fileName.lastIndexOf(".");
  if(period==-1){
    return fileName+dateFomat.format(new Date())+"-"+random.nextInt();
  }
  String basename = fileName.substring(0,period);
  String extension = filename.substring(period);
  return basename+dateFomat.format(new Date())+"-"+random.nextInt()+extension;
}

I often need functionallity like this and always did it worse than yours.