3
\$\begingroup\$

I'm attempting to write a bit of code that allows for easy and safe access to objects in a file. It seems to work great but I was curious if there was an easier way to do this or if Java already has something like this. If there isn't anything like this any critiques of my code would be appreciated as I'm trying to learn to be a better Java programmer.

The main objectives I have are...

  • Modularity (I want to be able to load any object type)
  • Type-safety (I want all the object returned to be of the correct type give no warnings)
  • Thread-safety (I want to be able to read and write multiple files, multiple times, concurrently)

loadFile is just a basic implementation of using a Loader, this code assumes those three files contain a Map object in the first, a Set object in the second, and a Set object in the third. You can either make those files using any standard method or else using the FileHandler class.

private void loadFiles() {
    long time = System.currentTimeMillis();

    Loader<Map<?, ?>, Map<String, Long>> loader1 = new Loader<Map<?, ?>, Map<String, Long>>(new File("file1.bin"), object1);
    Loader<Set<?>, Set<Integer>> loader2 = new Loader<Set<?>, Set<Integer>>(new File("file2.bin"), object2);
    Loader<Set<?>, Set<File>> loader3 = new Loader<Set<?>, Set<<File>>(new File("file3.bin"), object3);

    boolean OK1 = loader1.load();
    boolean OK2 = loader2.load();
    boolean OK3 = loader3.load();

    if(OK1) {
        object1 = loader1.getVariable();
    }
    if(OK2) {
        object2 = loader2.getVariable();
    }
    if(OK3) {
        object3 = loader3.getVariable();
    }

    System.out.println("File load took " + (System.currentTimeMillis() - time) + "ms.");
}

The FileHandler class is used to handle file reads and writes in a thread safe way.

public class FileHandler {

private static Set<File> fileSet = Collections.synchronizedSet(new HashSet<File>());

private final ReentrantReadWriteLock readWriteLock;
private final Lock read;
private final Lock write;
private final File file;

/**
 * Creates a new thread-safe FileHandler for reading and writing objects to a file. 
 * There can be at most one FileHandler associated with any given file, if a FileHandler is created 
 * that is to contain a File that another FileHandler already contains an IllegalArgumentException is thrown.
 * @param f - the file to be used for reading and writing.
 * @throws IllegalArgumentException
 */
public FileHandler(File f) throws IllegalArgumentException {
    if(!fileSet.add(f)) {
        throw new IllegalArgumentException("There is already a FileHandler associated with " + f.getName());
    }

    file = f;
    readWriteLock = new ReentrantReadWriteLock();
    read = readWriteLock.readLock();
    write = readWriteLock.writeLock();
}

/**
 * Loads an Object whose class is "type" from "file" and returns it if valid, else it returns null.
 * @param file - the file to be opened for reading.
 * @param type - the type of the object to be read.
 * @return the object that was loaded from the file.
 */
public <T> T loadObject(Class<T> type) {
    read.lock();
    try {
        InputStream is = new FileInputStream(file);
        ObjectInputStream ois = new ObjectInputStream(new BufferedInputStream(is));
        try {
            return type.cast(ois.readObject());
        } finally {
            ois.close();
        }
    } catch (FileNotFoundException e) {
        System.err.println("File load failed with error: " + e.getLocalizedMessage());
    } catch (IOException e) {
        System.err.println("File read failed with error: " + e.getLocalizedMessage());
    } catch (ClassNotFoundException e) {
        System.err.println("Object class definition failed with error: " + e.getLocalizedMessage());
    } catch (ClassCastException e) {
        System.err.println("Object cast failed with error: " + e.getLocalizedMessage());
    } finally {
        read.unlock();
    }
    return null;
}

/**
 * Saves an Object of the specified class to "file" and returns true on success.
 * @param file - the file to be opened for writing.
 * @param object - the object to be written to file.
 * @return true if the save was successful.
 */
public <T> boolean saveObject(T object) {
    write.lock();
    try {
        OutputStream os = new FileOutputStream(file);
        ObjectOutputStream oos = new ObjectOutputStream(new BufferedOutputStream(os));
        try {
            oos.writeObject(object);
        } finally {
            oos.close();
        }
        return true;
    } catch (FileNotFoundException e) {
        System.err.println("File save failed with error: " + e.getLocalizedMessage());
    } catch (IOException e) {
        System.err.println("File write failed with error: " + e.getLocalizedMessage());
    } finally {
        write.unlock();
    }
    return false;
}

@Override
protected void finalize() {
    fileSet.remove(file);
}
}

The FileLoader class is the class that does the typecasting and the actual loading of the object via a FileHandler.

public class FileLoader <G, F> implements Runnable {

private F variable = null;
private FileHandler fileHandler = null;

public FileLoader(FileHandler fh, F var) {
    fileHandler = fh;
    variable = var;
}

public F getVariable() {
    return variable;
}

@SuppressWarnings("unchecked")
@Override
public void run() {
    G temp = (G) fileHandler.loadObject(variable.getClass());
    variable = (F) temp;
}
}

Loader is the class that loads the object from the file. It has two type parameters, the first being the generic type of the Object you want to load and the second being the formal type of the object. It takes in a file and an "example object". The example object cannot be null and must have the same type as the object you want to load. The Loader class is the class that is generally used to load the objects from file and contains the high level methods that make use of the other classes.

public class Loader <G, F> {

private File file = null;
private FileHandler handler = null;
private FileLoader<G, F> fileLoader = null;
private Thread thread = null;
private F variable = null;

/**
 * Creates a new Loader. A Loader is used for loading an object from a file with type-safety and thread-safety.
 * @param f - The file to be loaded from.
 * @param var - The object to load from the file.
 */
public Loader(File f, F var) {
    file = f;
    variable = var;
}

/**
 * Attempts to start loading an object from the file.
 * @return true if the load started successfully or false if the file cannot be loaded.
 */
public boolean load() {
    if(file.exists()) {
        try {
            handler = new FileHandler(file);
        } catch (IllegalArgumentException e) {}

        if(handler != null) {
            fileLoader = new FileLoader<G, F>(handler, variable);
            thread = new Thread(fileLoader);
            thread.start();
            return true;
        }
    }
    return false;
}

/**
 * This blocking method waits for the Loader thread to finish and then returns its variable.
 * If the Loader thread has not started or is null this method returns null.
 * @return the value loaded from the file.
 */
public F getVariable() {
    if(thread != null && !thread.getState().equals(Thread.State.NEW)) {
        try {
            thread.join();
        } catch (InterruptedException e) {}
        return fileLoader.getVariable();
    }
    return null;
}
}
\$\endgroup\$
2
\$\begingroup\$
  1. The Loader class could use Java's built-in ExecutorService with a Callable and a Future instead of reinventing the wheel. See: Effective Java Second Edition, Item 47: Know and use the libraries

  2. Instead of printing error messages you should use a logger framework (for example, SLF4J and Logback). See: log4j vs. System.out.println - logger advantages?

Some notes about the current code:

  1. The static fileSet field in the FileHandler class makes testing really hard. Actually, this class does not fulfill the Single Responsibility Principle. It loads/saves the objects and stores the handled files. I'd separate it to two class. One (FileStorage, for example) is responsible for loading and saving and another class creates FileStorage instances and checks that there is no more than one FileStorage instance for the same file.

  2. Do not rely on the finalize method. Make the releasing explicit. For example, create a close() method in the FileHandler which unregisters the file and sets a flag/sets the file field to null. Then check the flag in the loadObject and saveObject methods. If the flag is set throw an IllegalStateException. See Effective Java Second Edition, Item 7: Avoid finalizers.

  3. In the Loader.load method I'd use guard clauses:

    public boolean load() {
        if (!file.exists()) {
            return false;
        }
        try {
            handler = new FileHandler(file);
        } catch (IllegalArgumentException e) {
            return false;
        }
    
        fileLoader = new FileLoader<G, F>(handler, variable);
        thread = new Thread(fileLoader);
        thread.start();
        return true;
    }
    
  4. Consider throwing a non-RuntimeException (a custom Exception subclass, not an IllegalArgumentException) when a client tries to create a FileHandler multiple times for the same file. Another idea is returning the former FileHandler again instead of throwing an exception, since FileHandler is thread-safe.

  5. You should check parameters for validity. The majority of the methods and constructors should check at least null input references. See Effective Java Second Edition, Item 38: Check parameters for validity

\$\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.