HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Saving User Preferences

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
userpreferencessaving

Problem

I have the following given code after some refactoring:

public static void storePreferences() {
    try {
        Preferences.userRoot().exportSubtree(new FileOutputStream(ReleaseInfo.getAppFolder() + File.separator + SETTINGSFILENAME));
    } catch (IOException e) {
        e.printStackTrace();
    } catch (BackingStoreException e) {
        e.printStackTrace();
    }
}


Now I wonder - should I explicitly close the FileOutputStream or the JVM takes care for doing so? I have looked at this question on the same topic, where it is said that it is not a good practice to rely on the garbage collector, but still I need a bit more confirmation (therefore this question). I have changed the above code to:

public static void storePreferences() {
    FileOutputStream fos = null;

    try {
        fos = new FileOutputStream(ReleaseInfo.getAppFolder() + File.separator + SETTINGSFILENAME);
        Preferences.userRoot().exportSubtree(fos);
    } catch (IOException | BackingStoreException e) {
        e.printStackTrace();
    } finally {
        try {
            fos.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}


Here I close the stream manually, so which would be the better alternative and do the performed changes really bring something to the table?

Any other remarks?

Solution

The better alternative is the second alternative, because the first alternative may only write the data to disk when the Garbage Collector clears up the stream. The file will remain open for all that time too, leading to long holds on file handles in the OS.

i.e. the first one may produce unexpected results.

The second alternative is an improvement, but you should consider using a "try with resources" block, which has been around for the past 6 years in Java 7 and 8... The try-with-resources is a far more elegant way to approach this problem, and it makes the exception handling "trivial"...

public static void storePreferences() {
    File prefs = new File(new File(ReleaseInfo.getAppFolder()), SETTINGSFILENAME);
    try (OutputStream fos = new FileOutputStream(prefs)) {
        Preferences.userRoot().exportSubtree(fos);
    } catch (IOException | BackingStoreException e) {
        e.printStackTrace();
    }
}


Note that I used the more convenient 2-argument File constructor instead of the constant File.separator

Further, I would suggest getting a better-performing new NIO2 functions, and wrap the resulting OutputStream with a buffer:

public static void storePreferences() {
    Path prefs = Paths.get(ReleaseInfo.getAppFolder(), SETTINGSFILENAME);
    try (OutputStream fos = new BufferedOutputStream(Files.newOutputStream(prefs))) {
        Preferences.userRoot().exportSubtree(fos);
    } catch (IOException | BackingStoreException e) {
        e.printStackTrace();
    }
}

Code Snippets

public static void storePreferences() {
    File prefs = new File(new File(ReleaseInfo.getAppFolder()), SETTINGSFILENAME);
    try (OutputStream fos = new FileOutputStream(prefs)) {
        Preferences.userRoot().exportSubtree(fos);
    } catch (IOException | BackingStoreException e) {
        e.printStackTrace();
    }
}
public static void storePreferences() {
    Path prefs = Paths.get(ReleaseInfo.getAppFolder(), SETTINGSFILENAME);
    try (OutputStream fos = new BufferedOutputStream(Files.newOutputStream(prefs))) {
        Preferences.userRoot().exportSubtree(fos);
    } catch (IOException | BackingStoreException e) {
        e.printStackTrace();
    }
}

Context

StackExchange Code Review Q#158618, answer score: 2

Revisions (0)

No revisions yet.