Каков наилучший способ исправить это предупреждение findbugs «запись в статическое поле из метода экземпляра»?

У меня есть класс, похожий на этот, и findbugz жалуется на «запись в статическое поле из метода экземпляра» (initialize() и killStaticfield()). Я не могу установить статическое поле в ctor.

  • Как лучше всего решить эту проблему?

  • Достаточно ли будет поместить staticField в AtomicReference?

     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;
  }
}

java findbugs
person darrickc    schedule 02.09.2010    source источник
comment
Чтобы ответить на ваш вопрос, это поле является статическим, потому что метод get должен быть статическим, чтобы другие объекты могли получить доступ к staticField без ссылки на объект Something.   -  person darrickc    schedule 03.09.2010
comment
По сути, мой вопрос заключается в том, как лучше всего исправить предупреждение findbugz «запись в статическое поле из метода экземпляра»; Я только что составил код для представления предупреждения. Лучше ли обернуть статический объект в объект AtomicReference или синхронизировать?   -  person darrickc    schedule 03.09.2010

Ответы (4)


arrow_upward
16
arrow_downward

Оставаясь максимально близким к вашему первоначальному дизайну...

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;
  }
}

Обратитесь к своей статической переменной через имя класса, что удалит предупреждение findbugz. Пометьте свою статическую переменную как volatile, что сделает ссылку более безопасной в многопоточной среде.

Еще лучше было бы:

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

  public Something() {
  }

  public static SomeClass getStaticField() {
    return Something.staticField;
  }
}
person romacafe    schedule 02.09.2010
comment
Но дизайн без synchronized также означает отсутствие killStaticField. - person extraneon; 03.09.2010
comment
Вы знаете... Глядя на первое решение, оно все еще не является потокобезопасным, даже если переменная изменчива. Вам нужно будет поместить блок synchronized{} вокруг раздела if-null-then-new... Я бы определенно предпочел второй вариант. - person romacafe; 03.09.2010
comment
Я думаю (к сожалению) первое решение является правильным ответом в моем случае. В основном потому, что я не могу инициализировать staticField вверху, его нужно инициализировать после вызова initialize() (я отредактирую вопрос, чтобы отразить это). - person darrickc; 03.09.2010
comment
Если бы мы использовали 1-й блок кода, нам пришлось бы синхронизировать весь доступ к staticField вокруг SomeClass.class. - person darrickc; 03.09.2010
comment
Я изменил его ссылку, используя ClassName.staticField, однако поисковые ошибки по-прежнему жалуются на то же самое. - person Tiina; 09.03.2018

arrow_upward
4
arrow_downward

Удалите static из staticField, если он не должен быть статичным.

Сделайте kill и getStaticField статическими. И вы обычно ссылаетесь на static по имени класса, а не по (неявному) this, чтобы было ясно, что это static и может вызвать неожиданные последствия в других thReads.

Если вы сомневаетесь, не используйте статику для непостоянных полей.

person extraneon    schedule 02.09.2010
comment
Вам не нужно Когда в сомнении. - person Tom Hawtin - tackline; 03.09.2010

arrow_upward
4
arrow_downward

Вопрос в том, что вы хотите делать со статическим полем. Если он меняется для каждого создаваемого вами класса, возможно, не стоит вообще делать его статическим. Если он инициализируется только один раз, вы должны просто лениво инициализировать его как синглтон.

public class Something
{
    private static SomeClass staticField = null;

    public Something()
    {

    }

    public static SomeClass getStaticField()
    {
        if(staticField == null)
            staticField = new SomeClass();;
        return staticField;
    }
}
person Daff    schedule 02.09.2010
comment
Не забудьте сделать этот getStaticField метод synchronized! - person Richard Fearn; 02.09.2010
comment
И заметьте, вы не можете просто поместить синхронизированный в определение статического метода, так как статические методы используют Something.class для синхронизации! - person darrickc; 03.09.2010
comment
Я думаю, что использование статических переменных должно быть ясно, прежде чем обсуждать какие-либо сложные вопросы потоковой передачи. И извините, но ваш вопрос на самом деле не звучит так, как будто вы поняли, когда использовать статические переменные, а когда нет. Может быть, вы можете немного уточнить свой вопрос. - person Daff; 03.09.2010
comment
@darrickc Что не так с синхронизацией в классе? Если ваш класс является только держателем синглтона (как ваш пример), это достойное решение для синхронизации. - person extraneon; 03.09.2010

arrow_upward
0
arrow_downward

Лучший способ - не делать этого, попытаться найти лучший дизайн-паттерн. Если это действительно необходимо, это сработает и заставит findbugs/spotbugs не жаловаться.

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;
    }
}
person The CTO    schedule 08.06.2020