Is this the correct way to use an instance variable in Java?

I have a class that reads a configuration file once and puts all the parameters (key/value pairs) in a Map.

For retrieving values from the Map instance I have created a method that takes a key and return the corresponding value开发者_运维技巧. The config map is passed to all the classes that need to read from the configuration. Each class that uses it reads the configuration parameters it needs and stores them in instance variables which are referred to in the rest of the class.

Is it correct to use an instance variable and store the value in that or should I call the accessor method on the config object whenever I am required to get the value for any key?

First, your configuration class should consider using java.util.Properties for its loading (and saving if necessary.) It's very convenient.

Second, what you're doing is dependency injection, which is a good thing. You make it clear that "This class uses --THESE-- configuration settings." I think what you're doing is right.

It sounds like you're caching these values in your class rather than looking them up from your configuration. That's fine under the following conditions

1. The configuration won't change at runtime. OR
2. If the configuration DOES change, you don't want to change the instance of the class.

Example of two:

void spawnClass() {
   MyProps config = magicConfigurationLoad();
   MyClass a = new MyClass(config); // caches property xyz
   config.setProperty("xyz","bbq"); // change property xyz
   MyClass b = new Class(config); // caches the new value of xyz
}

In any event, this is why you only reference instance variables in your get() methods and always use get() methods, even in internal methods.

Consider the internal workings of MyClass

class MyClass {

   static final String IMPORTANT_PROP_KEY = "xyz";
   String myProp;

   MyClass(MyProps props) {
       this.myProp = props.get(IMPORTANT_PROP_KEY);
   }

   void myFunc() {
       System.out.println("My prop is " + myProp); // bad
   }

}

Now let's say you want to change the implementation of MyClass so it references a MyProps instance so it can change on the fly (as in, #2 above doesn't apply, you want to change behavior as you go

class MyClass {
    static final String IMPORTANT_PROP_KEY = "xyz";
    MyProps props;

    MyClass(MyProps props) {
       this.props = props;
   }

   void myFunc() {
       System.out.println("My prop is " + props.get(IMPORTANT_PROP_KEY)); // ugly
   }

}

The class would have been better from the start if we made our class use the getter method, even if it's private. Getters do not need to be public if you don't want

class MyClass {

   static final String IMPORTANT_PROP_KEY = "xyz";
   String myProp;

   MyClass(MyProps props) {
       this.myProp = props.get(IMPORTANT_PROP_KEY);
   }

   void myFunc() {
       System.out.println("My prop is " + getMyProp()); // good
   }

   private String getMyProp() {
       return myProp; // good
   }

}

This way when we go to change the behavior to be not cached, but instead dynamic to the props file, we can do this:

class MyClass {
    static final String IMPORTANT_PROP_KEY = "xyz";
    MyProps props;

    MyClass(MyProps props) {
       this.props = props;
   }

   // this function didn't change
   void myFunc() {
       System.out.println("My prop is " + getMyProp()); // good
   }

   private String getMyProp() {
          return props.get(IMPORTANT_PROP_KEY); // good
   }

}

Now granted, this was a small example with one instance. Most of your classes will have 20 or 30 places where you'd have to change it. This makes it much easier to refactor your code, even if the getters are private.

It sounds exactly like what I've done many times. You might also think about how to handle changes to the configuration and, if it changes, writing back out to the file.

You can use a static map with static helper method.

class SYstemConfiguration{
 private static Map<String,Object> map;

 public static Object getConfiguration(String key){
  /*some other processing*/
  return map.get(key);
 }
}

Honestly I would just make the configuration object/method static. So instead of passing an instance to every object that might need configuration, simply create something like this

public class Configuration{
  private static Map<String, Object> configMap = new HashMap<String, Object>();
  public static Object getValue(String key){
    return configMap.get(key);
  }
}

Then, any time you need to use it, simply call

Configuration.getValue(someKey);

This would make life a lot easier later on in that you don't have to worry about keeping track of the instance of the configuration object.

I think you can make the Configuration object using the Singleton Design Pattern . There is only one Configuration instance exist and you use this instance to get the parameter value throughout the whole application . I goggle it and found this link may help you http://www.javenue.info/post/40