views:

1212

answers:

4

I inherited an application which uses a java properties file to define configuration parameters such as database name.

There is a class called MyAppProps that looks like this:

public class MyAppProps {

   protected static final String PROP_FILENAME = "myapp.properties";
   protected static Properties myAppProps = null;

   public static final String DATABASE_NAME = "database_name";
   public static final String DATABASE_USER = "database_user";
   // etc...

   protected static void init() throws MyAppException {
     try {
       Classloader loader = MyAppException.class.getClassLoader();
       InputStream is = loader.getResourceAsStream(PROP_FILENAME);
       myAppProps = new Properties();
       myAppProps.load(is);
     } catch (Exception e) {
       threw new MyAppException(e.getMessage());
     }
    }

    protected static String getProperty(String name) throws MyAppException {
      if (props==null) {
        throw new MyAppException("Properties was not initialized properly.");
      }
      return props.getProperty(name);
    }
  }

Other classes which need to get property values contain code such as:

String dbname = MyAppProps.getProperty(MyAppProps.DATABASE_NAME);

Of course, before the first call to MyAppProps.getProperty, MyAppProps needs to be initialized like this:

MyAppProps.init();

I don't like the fact that init() needs to be called. Shouldn't the initialization take place in a static initialization block or in a private constructor?

Besides for that, something else seems wrong with the code, and I can't quite put my finger on it. Are properties instances typically wrapped in a customized class? Is there anything else here that is wrong?

+1  A: 

You can use either, a static block or a constructor. The only advice I have is to use ResourceBundle, instead. That might better suit your requirement. For more please follow the link below.

Edit: ResourceBundles vs Properties

Adeel Ansari
+3  A: 

If I make my own wrapper class like this; I always prefer to make strongly typed getters for the values, instead of exposing all the inner workings through the static final variables.

private static final String DATABASE_NAME = "database_name"
private static final String DATABASE_USER = "database_user"
public String getDatabaseName(){
   return getProperty(MyAppProps.DATABASE_NAME);
}
public String getDatabaseUser(){
   return getProperty(MyAppProps.DATABASE_USER);
}

A static initializer looks like this;

static {
   init();
}

This being said, I will readily say that I am no big fan of static initializers.

You may consider looking into dependency injection (DI) frameworks like spring or guice, these will let you inject the appropriate value directly into the places you need to use them, instead of going through the indirection of the additional class. A lot of people find that using these frameworks reduces focus on this kind of plumbing code - but only after you've finished the learning curve of the framework. (DI frameworks are quick to learn but take quite some time to master, so this may be a bigger hammer than you really want)

krosenvold
+1  A: 

Reasons to use static initializer:

  • Can't forget to call it

Reasons to use an init() function:

  • You can pass parameters to it
  • Easier to handle errors

I've created property wrappers in the past to good effect. For a class like the example, the important thing to ensure is that the properties are truly global, i.e. a singleton really makes sense. With that in mind a custom property class can have type-safe getters. You can also do cool things like variable expansion in your custom getters, e.g.:

myapp.data.path=${myapp.home}/data

Furthermore, in your initializer, you can take advantage of property file overloading:

  • Load in "myapp.properties" from the classpath
  • Load in "myapp.user.properties" from the current directory using the Properties override constructor
  • Finally, load System.getProperties() as a final override

The "user" properties file doesn't go in version control, which is nice. It avoids the problem of people customizing the properties file and accidentally checking it in with hard-coded paths, etc.

Good times.

Dave Ray
+1  A: 

The problem with static methods and classes is that you can't override them for test doubles. That makes unit testing much harder. I have all variables declared final and initialized in the constructor. Whatever is needed is passed in as parameters to the constructor (dependency injection). That way you can substitute test doubles for some of the parameters during unit tests.

For example:

public class MyAppProps {

   protected static final String PROP_FILENAME = "myapp.properties";
   protected Properties props = null;

   public String DATABASE_NAME = "database_name";
   public String DATABASE_USER = "database_user";
   // etc...

   public MyAppProps(InputStream is) throws MyAppException {
     try {
       props = new Properties();
       props.load(is);
     } catch (Exception e) {
       threw new MyAppException(e.getMessage());
     }
    }

    public String getProperty(String name) {
      return props.getProperty(name);
    }
    // Need this function static so
    // client objects can load the
    // file before an instance of this class is created.
    public static String getFileName() {
      return PROP_FILENAME;
    }
}

Now, call it from production code like this:

String fileName = MyAppProps.getFileName();
Classloader loader = MyAppException.class.getClassLoader();
InputStream is = loader.getResourceAsStream(fileName);
MyAppProps p = new MyAppProps(is);

The dependency injection is when you include the input stream in the constructor parameters. While this is slightly more of a pain than just using the static class / Singleton, things go from impossible to simple when doing unit tests.

For unit testing, it might go something like:

@Test
public void testStuff() {
    // Setup
    InputStringTestDouble isTD = new InputStreamTestDouble();
    MyAppProps instance = new MyAppProps(isTD);

    // Exercise
    int actualNum = instance.getProperty("foo");

    // Verify
    int expectedNum = 42;
    assertEquals("MyAppProps didn't get the right number!", expectedNum, actualNum);
}

The dependency injection made it really easy to substitute a test double for the input stream. Now, just load whatever stuff you want into the test double before giving it to the MyAppProps constructor. This way you can test how the properties are loaded very easily.

Master1588