3

My code currently suffers from "Possible unsafe assignment to a non-final static field in a constructor" (AssignmentToNonFinalStatic in PMD).

The class is written as a singleton class, the property affected by this warning looks like this

private static String myProperty;

and is filled by this construct:

public SystemPropertyUtils() throws ConfigException {
    someMethodThrowingConfigException();
    myProperty = "someValue" + this.someOtherValueFromAThreadSafeString;
}

Is there an elaborated way to negate this warning?

Robert Heine
  • 1,820
  • 4
  • 29
  • 61

2 Answers2

5

Don't set static fields in the constructor. In this case, make the field, non-static.

Otherwise, I would have to suspect you don't need a constructor. Instead you can initialise the static field in a static initialiser block, or static method.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • Why not? What if -- trivial example -- I have an object instance counter that I want to increment inside the constructor to keep track of how many have been constructed? – simpleuser Mar 15 '19 at 17:52
  • 2
    @simpleuser incrementing a value is reasonable, though you should be using an `static final AtomicLong s_counter = new AtomicLong();` – Peter Lawrey Mar 15 '19 at 20:13
  • Late, and I don't understand well enough to add/edit an answer, but [this A to a similar Q](https://stackoverflow.com/a/2559622) fixed it for me. – l3l_aze Apr 15 '22 at 20:16
0

That sounds like a sane thing to do in some cases, for example when you create a singleton class in a framework at boot time (say a spring service or component) and would like to expose that instance via static functions.

Here the solution is to write a "static" setter function, and call that setter function from your constructor, instead of directly affecting the static variable. This won't trigger the PMD as far as I can tell...

E.g:

public class MySingletonClass {
    private static MySingletonClass theInstance;
   
    // Static setter
    private static setStaticInstanceValue(MySignletonClass theValue) {
        theInstance = theValue;
    }

    public MySingletonClass() {
        // Do whatever initialization you need
        // ...

        MySingletonClass.setStaticInstanceValue(this);
    }

    public static MySingletonClass getInstance() {
        return theInstance;
    }
}