Записывая статическую переменную в методе экземпляра, почему это плохая практика?

Я немного запутался здесь с предупреждением об ошибках в eclipse.

public class MyClass {
    public static String myString;
}


public class AnotherClass {
   public void doSomething() {
       MyClass.myString = "something";
   }
}

Это дает мне предупреждение findbugs "писать в статическое поле из метода экземпляра", однако это не дает мне предупреждения:

public class MyClass {
    public static String myString;
}


public class AnotherClass {
   public void doSomething() {
       doAnotherThing();
   }
   public static doAnotherThing() {
       MyClass.myString = "something";
   }
}

Как это иначе? Почему зачем писать статическую переменную из метода экземпляра плохую практику? Я предполагаю, что это связано с синхронизацией, но мне все еще не ясно.

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

Ответ 1

Его форма сглаживания, которая может быть противоинтуитивной. Контр-интуитивный код затрудняет обслуживание.

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

Переименуйте doSomething в initialize:

...
a.initialize();
...
b.initialize();
...

Читатель этого кода может не сразу понять, что экземпляры a и b фактически влияют на одни и те же данные. Это может быть ошибкой, так как мы инициализируем одну и ту же память дважды, но ее неочевидность, поскольку кажется разумным, что нам может потребоваться вызвать initialize для каждого экземпляра.

Однако код был:

...
MyClass.initialize();
...
MyClass.initialize();
...

В этом случае более интуитивно понятно, что мы, вероятно, затронем одни и те же статические данные, и это, вероятно, ошибка.

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

В последнем примере

экземпляр вызывает статический метод

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

В некотором смысле, он должен генерировать другое, но похожее предупреждение: один класс возится с данными другого класса. Однако, старая переменная public является способом молчаливого одобрения этого, поэтому такое предупреждение не требуется.

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

Ответ 2

Существует не так много случаев использования, почему вы хотите изменить статическое поле. Помните, что если вы установите для этого поля новое значение, которое это значение изменилось для всех экземпляров этого класса. Это может вызвать проблемы в многопоточной среде, где более чем один поток вызывает doSomething(). Требуется правильная синхронизация.

В 99% всех случаев вы хотите, чтобы ваши методы экземпляра изменяли нестатические поля, поэтому findbugs предупреждает вас.

И findbugs недостаточно умны, чтобы узнать о вашем методе экземпляра, косвенно изменяющем поле в вашем втором примере:)

Ответ 3

Это то, что говорит FindBugs об этом: http://findbugs.sourceforge.net/bugDescriptions.html#ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD

Ответ 4

Это мой прием, поэтому возьмите его с солью. Вы упомянули проблемы синхронизации, которые являются основной причиной этого предупреждения, но что более важно, эти два случая в основном работают на разных концептуальных "уровнях" данных. Методы экземпляра "принадлежат" объектам и изменяют данные, описывающие отдельные экземпляры. Методы класса - это общие операции и указывают, что, связанные с классом, не связаны с отдельными объектами. Таким образом, изменение этого состояния изнутри каждого экземпляра могло бы (но не обязательно) быть плохим дизайнерским решением.

Ответ 5

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

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

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

Ответ 6

Я не думаю, что синхронизация (упомянутая в нескольких ответах) имеет какое-либо отношение к этому. В конце концов, статические методы могут быть вызваны из нескольких потоков так же легко, как и методы экземпляра.

Причина предупреждения (не очень хорошо объясненная документацией FindBugs), я думаю, намекается на пару ответов: это подозрительно и, возможно, ошибка. Как сказал Йохен Бедерсдорфер, существует не так много случаев использования, когда вы хотите назначить статическую переменную в одном классе из метода экземпляра в другом. Так же, как

while (x = y) {
    // ...
}

не является технически ошибкой (и фактически легальной Java, если x и y являются логическими), это почти всегда ошибка. Точно так же авторы FindBug чувствовали то же самое и в рассматриваемом случае.