Статический метод get - это плохая практика?

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

У нас много картотекторов объектов базы данных и называются такими функциями

(пример) - метод setId получает строку из базы данных и устанавливает ее в предопределенные свойства

class Person {

    public static function get($id) {
        $object = new Person;
        $object->setId($id);
        return $object;
    }
}

Используя его таким образом, мы можем использовать простые конструкции, подобные этому: (где мы получили идентификатор для примера сообщения)

$person = Person::get($id);

вместо

$person = new Person;
$person->setId($id);

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

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

class CatalogArticle {
   public static function get($id) { }
   public static function getByArticlenumber($articlenumber) {} //$articlenumber is unique in the database
   public static function getRandom() {} //Runs a query returning a random row
}

Ответ 1

Это не ужасно. Это реализация шаблона дизайна Factory Method. Это совсем не плохо в принципе.

Однако в вашем конкретном примере это не делает ничего существенного, поэтому я не уверен, что это необходимо. Вы можете устранить необходимость, взяв (возможно, необязательный) параметр для конструктора для id. Тогда любой может вызвать $foo = new Person($id);, а не явно factory.

Но если инстанцирование сложное или вы хотите создать несколько разных типов людей, которые могут быть определены только логикой, метод factory может работать лучше. Например, скажем, вам нужно определить тип человека для создания экземпляра по некоторому параметру. Тогда был бы подходящим метод factory на Person. Метод определит, какой "тип" загружается, а затем создает экземпляр этого класса.

Статики в целом трудно проверить и не допускать полиморфных изменений, подобных экземпляру. Они также создают жесткие зависимости между классами в коде. Они не ужасны, но вы должны действительно думать об этом, если хотите его использовать. Вариантом будет использование Builder или Аннотация Factory. Таким образом, вы создаете экземпляр конструктора / factory, а затем пусть этот экземпляр определяет, как создать экземпляр полученного класса...

Еще одно примечание. Я бы переименовал этот метод из Person::get() в нечто более семантически подходящее. Возможно, Person::getInstance() или что-то еще подходящее.

Ответ 2

Это сообщение в блоге должно сказать вам, почему люди не любят статические методы лучше, чем я мог:

http://kore-nordmann.de/blog/0103_static_considered_harmful.html

Вопрос, который больше всего меня поражает в вашем текущем фрагменте кода: Является ли Person разрешенным НЕ иметь идентификатор?

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

Разница между двумя вызовами незначительна. Оба "создают" класс Person и устанавливают Id, чтобы вы не выигрывали/не теряли что-либо там, когда речь заходила о "жестких проводных зависимостях".

Преимущество показывает только, когда вы хотите передать Личность другому объекту и что объекты должны изменить ID (например, сообщение в блоге должно объяснить, что лучше, чем я сделал здесь).

Ответ 3

Я добавляю только к edorian post, но в прошлом использовал статические методы get, где есть механизм кэширования, и (например) у меня мог бы быть данный объект Person в memcache и скорее извлеките его из кеша, чем отправляйтесь в базу данных.

Например:

class Person {

    public static function get($id) {

        if(Cache::contains("Person", $id))
        {
           return Cache::get("Person", $id);
        }
        else
        {           
            //fictional get_person_from_database, basically
            //getting an instance of Person from a database
            $object = get_person_from_database($id);
        }
        return $object;
    }

}

Таким образом, вся обработка кеша выполняется рассматриваемым классом, а не вызывающим абонентом, вызывающим вызов пользователя, чтобы беспокоиться о кеше.

Ответ 4

длинный рассказ короткий, да, это плохая практика:

Хорошая причина в том, что вы должны "проверять свой код". Статические методы вызывают проблемы, поэтому у вас есть веская причина:

если вы хотите следовать рекомендациям, проверьте свой код
Ergo, если статические причины проблемы с тестированием, статические предупреждения не позволяют писать тесты, чтобы предотвратить следование передовым методам: -)

Ответ 5

время идет, все меняется.

на случай, если у вас возникнут проблемы с тестированием, вы можете использовать библиотеку AspectMock https://github.com/Codeception/AspectMock

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