1
\$\begingroup\$

I wrote the following code to read the config/property files. I will be exposing my api as jar to multiple application teams.

Functionality:

  • Reads the property files and returns the resource bundle object. I am returning the resource bundle object as, there can be multiple config files in the application.
  • Getting the properties or the configs as variables from the resource bundle and use it across the application.

    public class FileConfigLoader {
    
        private static final Logger LOGGER = Logger.getLogger(FileConfigLoader.class);
    
        private static String EMPTY = "";
    
        public static ResourceBundle getBundle(String basePath, String baseName) throws MalformedURLException {
    
            File file = new File(basePath);
            URL[] uris = { file.toURI().toURL() };
            ClassLoader loader = new URLClassLoader(uris);
            ResourceBundle rb = ResourceBundle.getBundle(basePath, Locale.getDefault(), loader);
            return rb;
        }
    
        public static String getValue(ResourceBundle rb, String key) {
            String value = EMPTY;
    
            try {
                value = rb.getString(key);
            } catch (MissingResourceException mREx) {
                LOGGER.error("Missing Resource : " + key);
            }
    
            return value;
        }
    
    }
    

I need code reviews on the following lines

  • Does my code looks reusable?
  • Do you think i am using the variable names and class names as per the java sandards?
  • If my code does not seems to be reusable, Please provide suggestions.

The below code is the demonstration of how my code used by applications in the realtime.

The config is the name of my config file.The file is located in the classpath.

import java.net.MalformedURLException;
import java.util.ResourceBundle;

import org.apache.log4j.Logger;

public class MainApp {

    private static final Logger LOGGER = Logger.getLogger(MainApp.class);
    public static void main(String[] args) {

        ResourceBundle resource = null;

        try {
            resource = FileConfigLoader.getBundle("config", "");
        } catch (MalformedURLException e) {
            e.printStackTrace();
        }

        String hive_db = FileConfigLoader.getValue(resource, "hive_db");
        String EDGE_HIVE_CONN = FileConfigLoader.getValue(resource, "EDGE_HIVE_CONN");
        String target_dir = FileConfigLoader.getValue(resource, "target_dir");
        String somekey = FileConfigLoader.getValue(resource, "somekey");

        LOGGER.info(hive_db);
        LOGGER.info(EDGE_HIVE_CONN);
        LOGGER.info(target_dir);
        LOGGER.info(somekey);
    }

}
\$\endgroup\$
  • \$\begingroup\$ Do you need internationalization in your app? What is the reason of using ResourceBundle? \$\endgroup\$ – Dimitri Mar 17 '16 at 11:33
  • \$\begingroup\$ yes, we need internationalization. \$\endgroup\$ – wandermonk Mar 17 '16 at 13:31
2
\$\begingroup\$

A couple points:

  • Encapsulate the resource bundle in an instance of the class instead of using static methods.

  • If the key doesn't exist, don't return the empty string. If this situation is acceptable, return null or change the return type to Optional<String> and return Optional.none(). If this situation indicates an error, throw an exception.

  • Don't declare MalformedURLException. That is an implementation detail. Catch and throw a more appropriate exception.

\$\endgroup\$
  • \$\begingroup\$ Sure, these points help. \$\endgroup\$ – wandermonk Mar 17 '16 at 13:32
1
\$\begingroup\$

The getBundle doesn't use the second parameter baseName, so you could safely remove it.

As @Solomonoff's Secret wrote, it would be better to encapsulate the resource bundle in a member field of the class. That way getValue could take a single key parameter, without the burden of carrying the resource bundle around.

It might be good to add a default value parameter to the getValue method. That way, instead of returning empty string when a key is not found, callers will have a way to specify their preferred default value.

In the demo code, you catch MalformedURLExceptionand just print the stack trace it in case it's thrown. But after printing the stack trace, the execution continues, when it shouldn't. It would be better to not catch the exception, declare throws MalformedURLException on the method, and let it crash the program when such failure happens, not executing any further (potentially causing damage).

\$\endgroup\$
  • \$\begingroup\$ How do i encapsulate my resource bundle. I want the resource bundle to be available when the class loads. Could you please provide an example. \$\endgroup\$ – wandermonk Mar 18 '16 at 2:14

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.