CODEWITHSUNDEEP
java
user1595858
user1595858

Reputation: 3878

private constructor for static methods

I have created the following class to validate certain values with constants. Why i am getting the following error? As the class need not to be initiated to use static methods, but still why it is trying to initiate. I am using java 1.6 Is this a good practice to do ? 

public final class Approver{

    // Avoids initiating this class
    private Approver() {
    }


    private static final List<String> APPROVED_LENGTH= new ArrayList<String>() {
        {
            addAll(KM_APPROVED_LIST);
            addAll(LM_APPROVED_LIST);
        }
    };

    private static final List<String> KM_APPROVED_LIST = new ArrayList<String>() {
        {
            add("L");
            add("G");
                    // so on
        }

    };
    private static final List<String> LM_APPROVED_LIST = new ArrayList<String>() {
        {
            add("P");
            add("K");
                    // so on
        }

    };
    public static boolean isApproved(String lenth) {
        return APRROVED_LENGTH.contains(length);
    }

From another class

if(Approver.isApproved("K"))

{......}

error

Caused by: java.lang.NoClassDefFoundError: Could not initialize class ...Approver.class

Upvotes: 1

Views: 677

Answers (2)

Jon Skeet
Jon Skeet

Reputation: 1500055

If you'd looked at the rest of the error, I think you've had seen what's wrong. In this statement:

private static final List<String> APPROVED_LENGTH= new ArrayList<String>() {
    {
        addAll(KM_APPROVED_LIST);
        addAll(LM_APPROVED_LIST);
    }
};

you're using KM_APPROVED_LIST and LM_APPROVED_LIST while they're both null... which means you're calling addAll(null) which will throw a NullPointerException.

For example, here's the exception I see in a short test app:

Exception in thread "main" java.lang.ExceptionInInitializerError
        at Test.main(Test.java:43)
Caused by: java.lang.NullPointerException
        at java.util.ArrayList.addAll(Unknown Source)
        at Approver$1.<init>(Test.java:14)
        at Approver.<clinit>(Test.java:12)
        ... 1 more

At that point, it should be pretty clear what's going on.

It would be cleaner to initialize everything in a static block, IMO - it takes all the ordering concerns away - as well as avoiding the nasty anonymous classes:

private static final List<String> APPROVED_LENGTH;
private static final List<String> KM_APPROVED_LIST;
private static final List<String> LM_APPROVED_LIST;

static {
    KM_APPROVED_LIST = new ArrayList<String>();
    KM_APPROVED_LIST.add("L");
    KM_APPROVED_LIST.add("G");
    LM_APPROVED_LIST = new ArrayList<String>();
    LM_APPROVED_LIST.add("P");
    LM_APPROVED_LIST.add("K");
    APPROVED_LENGTH = new ArrayList<String>();
    APPROVED_LENGTH.addAll(KM_APPROVED_LIST);
    APPROVED_LENGTH.addAll(LM_APPROVED_LIST);
}

Alternatively, you could reorder the fields and rely on the static variable initializer ordering - but preferably using Guava to make the code much clearer:

private static final List<String> KM_APPROVED_LIST =
    Lists.newArrayList("L", "G");
private static final List<String> LM_APPROVED_LIST =
    Lists.newArrayList("P", "K");
private static final List<String> APPROVED_LENGTH =
    Lists.newArrayList(Iterables.concat(KM_APPROVED_LIST, LM_APPROVED_LIST));

I should point out that just reordering the field declarations so that APPROVED_LENGTH is declared last fixes the problem - but it's still not nice code as-is, IMO.

You might also want to consider making these immutable lists, too.

Upvotes: 6

parsifal
parsifal

Reputation: 15

The error is happening because you're trying to create an anonymous inner class in a static context:

private static final List<String> APPROVED_LENGTH= new ArrayList<String>() {
    {
        addAll(KM_APPROVED_LIST);
        addAll(LM_APPROVED_LIST);
    }
};

I'm surprised that the compiler allows this to happen, but I suppose it doesn't do a lot of semantic checks (ie, verifying that the class can be instantiated). Regardless, here's how to fix it (and a better way to initialize static lists/maps in general):

private static final List<String> APPROVED_LENGTH= new ArrayList<String>();
static {
    addAll(KM_APPROVED_LIST);
    addAll(LM_APPROVED_LIST);
};

Upvotes: -1

Related Questions