Design issue on static initialization block

Question

I have a design question: let me expalin in simple example:

Public class A()
{
public static HashMap map = new HashMap();
public static String url = "default";
static {
  getJson();
}

//url getters and setters are defined

public static getJson() {
//code which uses url to get json and populate hashmap
}
public string getresult(String key) {
//uses hashmap to send result.
}

I am using static initialization block because i want to get json only once.

public class B {

//here I want to change url and call getJson method. If i call A.setUrl() then before setting url, A.getJson () method is called as it is in static initialization block.how can i set url first and then call getJson().

//is this a bad design?

}

Anything is a mutable static is likely to be very bad design. If you only want a single instance of something in a particular context, only construct it once in that context. — Tom Hawtin - tackline, Aug 06 '12 at 19:28
As an aside, are you using [raw types](http://stackoverflow.com/a/2770692/150884) because you're targeting Java versions older than 1.5? — hertzsprung, Aug 06 '12 at 20:02

Ajay George · Answer 1 · 2012-08-06T19:58:10.010

This should work I guess. Add a new method.

public static void getJson(String url) {
setUrl(url);
getJSon();
}

Static Initializers are generally a bad idea because unit testing becomes difficult .

Check out Misko Hevery's Guide to writing Testable Code.

You can rework the design by doing something like this :

public class A {
  //Add generics
  private Map map = new HashMap();
  public A(Map map){
    this.map = map;
  }
  public String getresult(String key) {
  //uses hashmap to send result.
  }

}

//Helper Class
public class URLToJSon() {
//Add private constructor
  public static Map convertUrlToJSon(String url) {
   //do the conversion and return a hashmap
  }
}

In this manner we get to follow the Single Responsibility Principle.

Now both the classes are testable as well.

score 2 · Accepted Answer · answered Aug 06 '12 at 20:21

Yes, it is bad design:

It is impossible to customize where A gets its data from without modifying the definition of A. Among other things, this prevents unit testing (as you probably don't want to fail the unit test if the network is unavailable ...).
If initialization fails (for instance because the remote URL is currently unavailable), you can't easily catch that exception, as you don't know which access triggered loading. You can't throw a checked exception from a static initializer. You can't retry initialization either (all subsequent access immediately result in an exception).

If you must access A through a static field, I'd recommend:

public class A {
    private static Map<String, String> map;

    /** must be invoked before get is first called */
    public static void init(Map<String, String> newmap) {
        map = newmap;
    }

    public static String get(String key) {
        return map.get(key);
    }
}

This separates the concern of using the data from the concern of obtaining it, allowing each to be replaced and tested independently.

Also consider getting rid of static, as it enforces there is only ever one map in your entire application at the same time, which is quite inflexible. (See the second code sample in Ajay's answer for how)

score 1 · Answer 3 · answered Aug 06 '12 at 19:27

1

Where is the URL set? In the constructor? If so, simply do

//Normal init stuff like set url here, followed by
if (! some check for if json is set) {
     setJson();
}

answered Aug 06 '12 at 19:27

Alex Coleman

7,216
1
22
31

Design issue on static initialization block

3 Answers3