If с проверкой присвоения — приемлемо?

Question

[Иван Иванов]

Есть у меня болька - люблю экспериментировать)))

public static function get($id)
{
    if (!$user_bank = self::findOneBy('`user_id` = ?', [
        $id
    ])) {
        $user_bank = new self();
        $user_bank->user_id = $id;
        $user_bank->deposit = 0;
        $user_bank->deposit_sum = 0;
        $user_bank->percent = 10;
        $user_bank->exchange_sum = 0;
        $user_bank->save();
    }
    return $user_bank;
}

Возможно ли такое использовать и нужно ли после ! дополнительно брать в скобки (потому что работает и без них)?

p.s. фанат не злись - я познаю мир)

Comments

[Михаил]

это не очевидный код, так что нет

[Иван Иванов]

Михаил, почему неочевидный? Выбираем данные из БД, если их нет, то добавляем и в любом случае возвращаем данные.

[Antonio Solo]

Иван Иванов, потому что при чтении этого кода другим (или вами через 7 лет) потребуется лишнее усилие что бы понять что там делается.

[Иван Иванов]

Antonio Solo, ну вроде все в этом вопросе упирают на читаемость, соглашусь

[nokimaro]

Использую иногда c json_decode() но опять же тут Yoda condition, а не просто !$obj

if(null !== $obj = json_decode($str))
{
    //$obj->...
}

[Иван Иванов]

nokimaro, ну иногда хочется внести что-то чтобы всех запутать, но это временный порыв)

Answer 1

[Antonio Solo]

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

Comments

[Иван Иванов]

читаемость - принято

Answer 2

[Никита Ремизов]

1) Часто при написании условий легко спутать = и ==. В данном случае стороннему разработчику придется понять, что это НЕ опечатка. Даже стандарт сделали, чтобы избежать таких ошибок, используется в Symfony, называется Yoda Conditions. https://en.wikipedia.org/wiki/Yoda_conditions#Erro...
2) Читать другому разработчику такую конструкцию не удобно, т.к. обычно ожидаешь в условии увидеть какое-то сравнение. А сейчас требуется разобраться, что мы одновременно присваиваем и делаем проверку.

Исходя из пунктов выше, я бы не рекомендовал использовать такие конструкции.
Альтернатива, сделать присвоение перед условием явно:

public static function get($id)
{
    $user_bank = self::findOneBy('`user_id` = ?', [$id]);
    if (!$user_bank) {
        $user_bank = new self();
        $user_bank->user_id = $id;
        $user_bank->deposit = 0;
        $user_bank->deposit_sum = 0;
        $user_bank->percent = 10;
        $user_bank->exchange_sum = 0;
        $user_bank->save();
    } 
    return $user_bank;
}

PS: Но в итоге ответ зависит от того, что для Вас значит "приемлемо":

* значит, что понятно другому разработчику? - В общем случае не приемлемо. В случае договоренности (стандартам) в команде/на проекте, приемлемо.
* значит что код, просто работает? - приемлемо.

PPS: Похожий вопрос на англ. языке https://stackoverflow.com/questions/2576571/assign...

Comments

[Иван Иванов]

В данном случае стороннему разработчику придется понять

принято) а то обратку потом закидают ЧАГО

Answer 3

[FanatPHP]

А чего тут злиться?
Нормальный вопрос на приведение типов и приоритет операторов.
И вообще, я злюсь когда тупят и отрицают очевидное. Вот как этот пассажир например. Хотя там скорее уже жалость, а не злость.

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

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

if (!$user_bank = self::findOneBy('`user_id` = ?', [$id]))
{

хотя опять же, для читабельности, лучше разнести на две строчки

$user_bank = self::findOneBy('`user_id` = ?', [$id]);
if (!$user_bank)
{

Comments

[Иван Иванов]

ок, я понял, всё упирается в читаемость

а массивы я просто когда-то приучился переносами писать, поэтому так, переучиться могу легко, просто это на автомате уже)