5
\$\begingroup\$

A couple of hours ago I asked a question on Stackoverflow to find out if there is a good way to delete files in a folder only if all files are indeed deletable and I got some good answers that led me (I think) in the right direction

Based on the two currently most upvoted answers I scrambled together the following code:

import java.io.File;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.nio.channels.FileLock;
import java.util.ArrayList;
import java.util.List;

public class Main {

    public static void main(String[] args) {
        File directory = new File("T:\\download\\test\\1234");

        if (safeDelete(directory)) {
            System.out.println("All deleted");
        } else {
            System.out.println("All kept");
        }
    }

    private static boolean safeDelete(File directory) {
        if (directory == null || !directory.isDirectory()
                || !directory.canWrite()) {
            return false;
        }

        // First get a lock on the directory itself
        FileLock dirLock;
        if ((dirLock = getLock(directory)) == null) {
            return false;
        }

        List<File> filesToDelete = new ArrayList<>();
        List<FileLock> fileLocks = new ArrayList<>();

        // Get a lock on all the files in the folder
        for (File file : directory.listFiles()) {

            FileLock lock;
            if (file.canWrite() && ((lock = getLock(file)) != null)) {
                filesToDelete.add(file);
                fileLocks.add(lock);
            } else {
                System.out.println("Currently unable to write to "
                        + file.getName());
                releaseLocks(fileLocks);
                return false;
            }
        }

        // If we reach this line all the files are locked for us now
        for (File file : filesToDelete) {
            if (!file.delete()) {
                System.out.println("IMPOSSIBLE");
            }
        }
        releaseLocks(fileLocks);

        directory.delete();
        releaseLock(dirLock);
        return true;
    }

    private static FileLock getLock(File file) {
        try {
            return new RandomAccessFile(file, "rw").getChannel().tryLock();
        } catch (IOException ex) {
            System.out.println(ex.toString());
            return null;
        }
    }

    private static void releaseLocks(List<FileLock> locks) {
        for (FileLock lock : locks) {
            releaseLock(lock);
        }
    }

    private static void releaseLock(FileLock lock) {
        if (lock != null) {
            try {
                lock.release();
                System.out.println("Released lock on " + lock.toString());
            } catch (IOException ex) {
                System.out.println("Cant release lock on " + lock.toString());
            }
        }

    }
}

But it seems so... bulky to me. There are many return statements in there (I personally like it better when there is only one at the very end), a lot of if-else branching, two ArrayLists that manage almost the same stuff and a lot of calls to the releaseLocks() method.

The core problem is in my opinion that I have to manage two sets of data on which I both have to operate:

  • The Files on which I have to execute the deletion
  • And the Locks which I have to release after I'm done

So the question is:
Can this be done more efficiently? Should I refactor the safeDelete() into more sub-methods?

\$\endgroup\$
  • \$\begingroup\$ Try to rename the whole directory, if that works, create a directory with the old name and delete the moved one. Of course assumes that the OS does not do something fishy with filelocks upon moving... \$\endgroup\$ – Bobby Jan 24 '14 at 12:20
  • \$\begingroup\$ you might want to put this as an answer on SO, right now I'm intrigued by the method above and I'd really appreciate a review on this :) \$\endgroup\$ – avalancha Jan 24 '14 at 12:24
3
\$\begingroup\$

Why not use a class to hold both the file and the lock, so you don`t need to manage two Lists like:

public class SafeDirectory {

    private File directory = null;
    private FileLock lock = null;
    private List<SafeFile> deleteableSafeFiles = new ArrayList<>();

    public SafeDirectory(String directoryPath) {
        directory = new File(directoryPath);
    }

    public boolean CanBeDeleted() {
        boolean canBeDeleted = false;
        if (directory != null && directory.canWrite() && directory.isDirectory()
                && ((lock = getLock(directory)) != null)) {

            canBeDeleted = true;
            for (File file : directory.listFiles()) {
                SafeFile safeFile = new SafeFile(file);

                if (safeFile.CanBeDeleted()) {
                    deleteableSafeFiles.add(safeFile);
                } else {
                    canBeDeleted = false;
                    System.out.println("Currently unable to write to "
                        + file.getName());
                    RelaseLock();
                    break;
                }
            }

        }
        return canBeDeleted;
    }

    public boolean Delete() {
        for (SafeFile safeFile : deleteableSafeFiles) {
            safeFile.Delete();
        }
        directory.delete();

        RelaseLock();
        return true;
    }

    private void RelaseLock() {
        for (SafeFile safeFile : deleteableSafeFiles) {
            releaseLock(safeFile.lock);
        }
        releaseLock(lock);
    }

    private boolean releaseLock(FileLock lock) {
        if (lock != null) {
            try {
                lock.release();
                System.out.println("Released lock on " + lock.toString());
                return true;
            } catch (IOException ex) {
                System.out.println("Cant release lock on " + lock.toString());
                return false;
            }
        }
        return true;

    }

    private static FileLock getLock(File file) {
        try {
            return new RandomAccessFile(file, "rw").getChannel().tryLock();

        } catch (IOException ex) {
            System.out.println(ex.toString());
            return null;
        }
    }

    private class SafeFile {

        private FileLock lock = null;
        private File file = null;

        SafeFile(File file) {
            this.file = file;
        }

        boolean CanBeDeleted() {
            return (file.canWrite() && ((lock = SafeDirectory.getLock(file)) != null));
        }

        boolean Delete() {
            return file.delete();
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.