Лучшая практика для обновления / записи в статическую переменную? - PullRequest
5 голосов
/ 16 декабря 2010

У меня есть проект, который отображает документацию отдела. Я храню все документы (получить из базы данных) в статическом arrayList. Каждый X час я перестраиваю этот arrayList на основе нового документа (если есть) из базы данных. Существует также статическая переменная для управления, чтобы перестроить этот массив или нет, устанавливать и сбрасывать в методе, который выполняет задачу перестроения. Каждый веб-браузер, попавший на сервер, будет создавать экземпляр этого класса, но doc arrayList и эта управляющая переменная являются общими для всех экземпляров класса.

Инструмент Find-Bugs жалуется, что «Запись в статическое поле someArrayName и someVariableName из метода экземпляра someClassMethod». Кажется, это не очень хорошая вещь (пусть метод экземпляра класса записывает в статическое поле). У кого-нибудь есть хорошая рекомендация, как обойти эту проблему? Спасибо.

Ответы [ 5 ]

7 голосов
/ 16 декабря 2010

По описаниям ошибок FindBugs :

ST: запись в статическое поле из метода экземпляра (ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD)

Этот метод экземпляра записывает в статическое поле. Это сложно сделать правильно, если манипулируют несколькими экземплярами, и, как правило, это плохая практика.

Помимо проблем параллелизма, это означает, что все экземпляры в JVM обращаются к одним и тем же данным и не разрешают две отдельные группы экземпляров. Было бы лучше, если бы вы имели одноэлементный объект «менеджер» и передавали его каждому экземпляру в качестве параметра конструктора или, по крайней мере, в качестве аргумента setManager() метода.

Что касается проблем параллелизма: если вы должны использовать статическое поле, ваше статическое поле должно быть окончательным; явная синхронизация сложна. (Есть также некоторые хитрые аспекты, если вы инициализируете неконечные статические поля, помимо моих знаний Java, но я думаю, что я видел их в книге Java Puzzlers.) Есть как минимум три способа справиться с этим (предупреждение следует непроверенный код, сначала проверьте перед использованием):

  1. Использование многопоточной коллекции , например Collections.synchronizedList обернут вокруг списка, к которому нет другого доступа.

    static final List<Item> items = createThreadSafeCollection();
    
    
    static List<Item> createThreadSafeCollection()
    {
       return Collections.synchronizedList(new ArrayList());
    }
    

    , а затем позже, когда вы заменяете эту коллекцию, из экземпляра:

    List<Item> newItems = getNewListFromSomewhere();
    items.clear();
    items.add(newItems);
    

    Проблема в том, что если два экземпляра выполняют эту последовательность одновременно, вы можете получить:

    Экземпляр1: items.clear (); Экземпляр2: items.clear (); Экземпляр1: items.addAll (newItems); Экземпляр2: items.addAll (newItems);

    и получите список, который не соответствует требуемому инварианту класса, а именно, что у вас есть две группы newItems в статическом списке. Таким образом, этот метод не работает, если вы очищаете весь список за один шаг и добавляете элементы как второй шаг. (Однако, если вашим экземплярам просто нужно добавить элемент, items.add(newItem) будет безопасно использовать из каждого экземпляра.)

  2. Синхронизировать доступ к коллекции.

    Вам понадобится явный механизм для синхронизации здесь. Синхронизированные методы не будут работать, потому что они синхронизируются по «this», что не часто встречается между экземплярами. Вы можете использовать:

    static final private Object lock = new Object();
    static volatile private List<Item> list;
    // technically "list" doesn't need to be final if you
    // make sure you synchronize properly around unit operations.
    
    
    static void setList(List<Item> newList)
    {
      synchronized(lock)
      {
          list = newList;
      }
    }
    
  3. использовать AtomicReference

    static final private AtomicReference<List<Item>> list;
    
    
    static void setList(List<Item> newList)
    {
      list.set(newList);
    }
    
1 голос
/ 16 декабря 2010

Если я правильно понимаю сообщение, отправленное вами из Find Bugs, это просто предупреждение.

Если вы хотите скрыть предупреждение, сделайте изменения статическим методом. Find Bugs предупреждает вас, потому что обычно это ошибка. Программист думает, что они изменяют некое состояние экземпляра, но на самом деле они меняют некое состояние, которое влияет на каждый экземпляр.

0 голосов
/ 16 декабря 2010

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

Посмотрите на это

http://download.oracle.com/javase/6/docs/api/index.html?java/util/concurrent/package-summary.html

в частности, часть о том, как ArrayList не синхронизируется. Также обратите внимание, что в пункте, который я упоминаю, есть решение, а именно

List list = Collections.synchronizedList(new ArrayList(...));

Это один из способов сделать это. Но это все еще не очень хорошая идея, а именно потому, что она может быть медленной. Если это не коммерческое приложение, и вы не имеете дело с большим объемом, вы, вероятно, можете получить, не делая его лучше. Если это тип приложения, которое срабатывает только несколько раз в день, вы можете игнорировать предупреждение, понимая, что возможно , что может произойти что-то плохое, если два запроса замкнут друг на друга.

Лучшее решение: поскольку у вас есть база данных, я буду просто получать информацию из БД по мере необходимости, т. Е. По мере поступления запросов. Вы можете использовать некоторые технологии кэширования для повышения производительности.

Причина, по которой мне не нравится идея синглтон-паттерна, заключается в том, что, даже если это заставляет предупреждение исчезнуть, оно само по себе не решает фундаментальную проблему синхронизации. Однако существует потокобезопасный http://en.wikipedia.org/wiki/Singleton_pattern#Traditional_simple_way_using_synchronization,, который может работать в этом случае.

0 голосов
/ 16 декабря 2010

Вам не нужно каждый раз удалять список.Как указано выше, вам придется иметь дело с несколькими потоками, но вы можете создать ArrayList один раз, а затем использовать методы clear () и addAll () для очистки и повторного заполнения.FindBugs должен быть вполне доволен этим, потому что вы не устанавливаете статическое значение.

ребята - не стесняйтесь вбрасывать, если есть какие-либо проблемы с этой техникой:управлять вещами из базы данных через Hibernate.Так что не ведите список, Hibernate имеет встроенное кэширование, так что это почти так же быстро.Если вы обновляете данные на уровне базы данных (что означает, что hibernate не знает), вы можете сказать hibernate очистить кеш и обновить базу данных при следующем запросе.

0 голосов
/ 16 декабря 2010

Использование шаблона проектирования Singleton является одним из способов. Вы можете иметь только один экземпляр объекта, который содержит желаемое значение, и обращаться к нему через глобальное свойство. Преимущество состоит в том, что, если вы хотите иметь больше экземпляров в дальнейшем, будет меньше модификаций ранее существовавшего кода (поскольку вы не меняете статические поля на поля экземпляров).

...