Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

Tags:

java

findbugs

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.

  • What is the best fix for this issue?
  • Would putting staticField in an AtomicReference suffice?

     public class Something
     {
      private static SomeClass staticField = null;
      private AnotherClass aClass;
      public Something()
      {
    
      }
    
      public void initialize()
      {
        //must be ctor'd in initialize
        aClass = new AnotherClass();
        staticField = new SomeClass( aClass );
      }
    
      public void killStaticField()
      {
       staticField = null;
      }
    
      public static void getStaticField()
      {
        return staticField;
      }
    }
    
like image 583
darrickc Avatar asked Sep 02 '10 19:09

darrickc


3 Answers

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;
  }
}
like image 147
romacafe Avatar answered Nov 16 '22 07:11

romacafe


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.

like image 29
extraneon Avatar answered Nov 16 '22 07:11

extraneon


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;
    }
}
like image 4
Daff Avatar answered Nov 16 '22 07:11

Daff