8

I have just spent half an hour figuring this thing out, I have managed to fix my code, but I don't fully understand what is going on and wondered if someone could shed some light on it.

I have a utils type class, that contains a few static fields (a database connection endpoint for example) that are used by various other programs depending on the task at hand. Essentially a library.

This is how it looked previously (while still broken);

//DBUtils.java
public final class DBUtils {

    private static DBConnection myDBConnection = spawnDBConnection();
    private static DBIndex myDBIndex = null;

    private static DBConnection spawnDBConnection() {
        //connect to the database
        //assign a value to myDBIndex (by calling a method on the DBConnection object) <- IMPORTANT
        //myDbIndex NOT NULL HERE
        System.out.println("database connection completed");
        //return the DBConnection object
    }

    public static searchDB(String name) {
        //use the myDBIndex to find a row and return it
    }
}

So briefly, I am using the static spawnDBConnection() method to assign a value to both myDBConnection and myDBIndex. This works perfectly, the first output line from my program is always "database connection completed", neither myDBConnection or myDBIndex are null at the end of the spawnDBConnection() method, everything is as it should be.

My external program looks like this;

//DoSomethingUsefulWithTheDatabase.java
public final class DoSomethingUsefulWithTheDatabase {

    public static void main(String args[]) {
        DBUtils.searchDB("John Smith"); //fails with NullPointerException on myDBIndex!
    }
}

This call to searchDB happens after the spawnDBConnection has finished, I have used the standard output extensively to show this. However, once inside the searchDB method, the value of myDBIndex is null! It's a static field, and it was not null by the end of spawnDBConnection, no other assignments have been made, and now it's null :(

The simple fix was to remove the "= null" so the field declaration now looks like;

private static DBIndex myDBIndex;

Why did that make a difference? I'm thoroughly confused by this one.

lynks
  • 5,599
  • 6
  • 23
  • 42
  • 4
    Consider making your statics `final`. You'll be forced to assign them exactly once, eliminating this sort of surprise. – Matt Ball Jul 24 '12 at 12:52
  • 4
    This is a terrible nightmare-antipattern what you are doing here. You are coupling class initialization to the acquisition of a database connection. – Marko Topolnik Jul 24 '12 at 12:54
  • You should not staticly initialize DBConnection. What if myDBConnection dies, are you going to start using DbUtils2 in your code? – Alexander Pogrebnyak Jul 24 '12 at 12:58
  • @MarkoTopolnik How would I go about making my DBUtils class as usable as it is (no code required by external programs, they can just jump in and start using the static methods) without making this kind of mess? – lynks Jul 24 '12 at 13:12
  • Database connection management is hard work and you should not start it from scratch. There are widely-used, battle-proven connection pool libraries out there. My favorite is `boneCP`. Download it and see, I think you'll find it quite easy to get started with. – Marko Topolnik Jul 24 '12 at 13:22

4 Answers4

14

That's because the assignment of null to myDBIndex is done after

private static DBConnection myDBConnection = spawnDBConnection();

e.g. overrides the assignment in spawnDBConnection

The sequence is:

  1. declare the fields myDBConnection, myDBIndex
  2. Initialize myDBConnection = spawnDBConnection();

    Which includes calling spawnDBConnection and assignment of the return value to myDBConnection

  3. Initialize myDBIndex (with null)

In your second example, the 3rd step does not exist.

MByD
  • 135,866
  • 28
  • 264
  • 277
  • Thanks for clarifying this, it makes perfect sense now. For some reason I had it in my head that any method calls would happen after 'simple' assignments. – lynks Jul 24 '12 at 13:21
7

Why did that make a difference? I'm thoroughly confused by this one.

The initializer for spawnDBConnection was running, then the initializer for myDBIndex was running. The initializer for myDBIndex set the value to null. As this happened after spawnDBConnection set it to a non-null value, the final result was that it was null.

Try not to do this - it's odd for a method called by a static initializer to set other static variables.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thanks for the reply, I knew when I was writing it that initializing one static variable within another was a weird thing to do, I was just throwing code together for some quick tests. – lynks Jul 24 '12 at 13:10
1

This is what happens in the generated static initalizer block:

static {
   myDBConnection = spawnDBConnection();
   myDBIndex = null;
}

I hope it is clear now.

Petar Minchev
  • 46,889
  • 11
  • 103
  • 119
0

As for as I know, if you define your method before your fields it does work, at initialization, class is parsed from top :

public class DbUtils {
    private static String spawnDBConnection() {
        System.out.println("database connection completed");
        return "INIT";
    }
    private static String myDBConnection = spawnDBConnection();
    private static int myDBIndex = 0;

    public static void main(final String[] args) {
        System.out.println(myDBConnection);
    }
}

Output :

database connection completed
INIT
cl-r
  • 1,264
  • 1
  • 12
  • 26