darrickc
darrickc

Reputation: 1932

What's the best way to fix this 'write to static field from instance method' findbugs warning?

I have a class that looks similar to this, and findbugz is complaining about the 'write to the static field from the instance method' (initialize(), and killStaticfield()). I can't set the static field in the ctor.

Upvotes: 19

Views: 63884

Answers (4)

The CTO
The CTO

Reputation: 91

The best way is not to do it, try to find a better design patern. If really necessary this will work and make findbugs/spotbugs not complain.

public class Something
{
    private static SomeClass staticField = null;

    public Something()
    {
    }

    private void setStaticField(SomeClass value){
        staticField=value;
    } 

    public static SomeClass getStaticField()
    {
        if(staticField == null)
            setStaticField(new SomeClass());
        return staticField;
    }
}

Upvotes: 0

Daff
Daff

Reputation: 44215

The question is what you want to do with the static field. If it changes for every class you create it might not be a good idea to have it static at all. If it gets initialized only once you should just lazily initialize it as a singleton.

public class Something
{
    private static SomeClass staticField = null;

    public Something()
    {

    }

    public static SomeClass getStaticField()
    {
        if(staticField == null)
            staticField = new SomeClass();;
        return staticField;
    }
}

Upvotes: 6

romacafe
romacafe

Reputation: 3128

Staying as close as possible to your original design...

public class Something {
  private static volatile SomeClass staticField = null;

  public Something() {
  }

  public static SomeClass getStaticField() {
    if(Something.staticField == null)
      Something.staticField = new SomeClass();;
    return Something.staticField;
  }
}

Refer to your static variable via the class name, that will remove the findbugz warning. Mark your static variable as volatile, which will make the reference safer in a multithreaded environment.

Even better would be:

public class Something {
  private static final SomeClass staticField = new SomeClass();

  public Something() {
  }

  public static SomeClass getStaticField() {
    return Something.staticField;
  }
}

Upvotes: 17

extraneon
extraneon

Reputation: 23960

Remove static from staticField if it should not be static.

Make kill and getStaticField static themselves. And you usually reference static by the class name, not by an (implicit) this, to make very clear that it is static and may cause unexpected consequences in other thReads.

When in doubt, don't use statics for non-constant fields.

Upvotes: 4

Related Questions