Attempting to make my properties an enum based singleton

Question

I was attempting to implement my first singleton using an enum, was a bit of a struggle for me and I'm not sure if its right and more importantly thread safe?

Property provider enum:

public enum PropertyProvider implements CanProvideProperties {

    INSTANCE;
    private final HashMap<String, String> properties;

    PropertyProvider() {
        properties = buildSettingsMap();
    }

    @Override
    public HashMap<String, String> getAllProperties() {
        return properties;
    }

    @Override
    public String getProperty(Property prop) {
        return properties.get(prop.toString());
    }

    @Override
    public HashMap<String, String> buildSettingsMap() {
        HashMap<String, String> temp = new HashMap<>();
        Properties prop = new Properties();
        try {
            prop.load(readFile("src/test/resources/framework.properties"));
        } catch (IOException ignore) {
            throw new RuntimeException("Framework properties file non accessible");
        }

        Enumeration<Object> KeyValues = prop.keys();
        while (KeyValues.hasMoreElements()) {
            String key = (String) KeyValues.nextElement();
            String value = prop.getProperty(key);
            temp.put(key, System.getProperty(key, value));
        }
        return temp;
    }

    @Override
    public FileInputStream readFile(String file) throws FileNotFoundException {
        return new FileInputStream(new File(file));
    }

}

Which implements the following interface:

public interface CanProvideProperties {
    String getProperty(Property prop);
    HashMap<String, String> getAllProperties();
    HashMap<String, String> buildSettingsMap();
    FileInputStream readFile(String file) throws FileNotFoundException;

}

and lastly another enum to hold some settings:

public enum Property {
    BROWSER {
        @Override
        public String toString() {
            return "browser";
        }
    },

    TESTENVIRONMENT {
        @Override
        public String toString() {
            return "env.baseurl";
        }
    },

    GRIDADDRESS {
        @Override
        public String toString() {
            return "grid.endpoint";
        }
    },

    LOCALORGRID    {
        @Override
        public String toString() {
        return "grid.or.local";
    }
    },

    PLATFORM {
        @Override
        public String toString() {
            return "platform";
        }
    },

    LANGUAGE {
        @Override
        public String toString() {
            return "application.language";
        }
    },

    PRODUCTNAME {
        @Override
        public String toString() {
            return "product.name";
        }
    },

    DATABASEURL {
        @Override
        public String toString() {
            return "database.url";
        }
    },

    BASEADMINACCOUNT {
        @Override
        public String toString() {
            return "base.admin.account";
        }
    },

    BASEADMINUSERNAME {
        @Override
        public String toString() {
            return "base.admin.username";
        }
    },

    BASEADMINPASSWORD {
        @Override
        public String toString() {
            return "base.admin.password";
        }
    },    
}

Answer 1

Property-enum

An enum can have a constructor, allowing us to shorten the code to setup all the enum values.

public enum Property {
    BROWSER("browser"),    
    TESTENVIRONMENT("env.baseurl"),
    ... // add more here.
    ;

    private final String key;

    Property(String key) {
        this.key = key;
    }

    public String getKey() {
        return this.key;
    }

    @Override public String toString() {
        return getKey();
    }
}

Do not have any logic based on toString. That method is mainly meant for debugging purposes. Using a getKey() instead (as above) allows you to use getKey() for your application logic.

Is a singleton really necessary?

Ask yourself if you really really NEED a singleton (Hint: Answer is most likely "no"). You might be able to use dependency injection to pass the object to the classes that needs it.

You might even not need a single instance, is there anything wrong with having multiple instances? All it does is read from a resource file, so worst case scenario is that you might spend a few nanoseconds reading a resource file from different places?