mr_MAIL Apr 9 2012 at 16:37

Assignments in conditions (присваивания в условиях)

2 min

17K

PHP*

-2

Comments 47

VolCh Apr 9 2012 at 16:56

Всё же я вынес бы $var = foo() за условие. Смысл его там держать? Тем более что точного равенства не используется и вполне допустимо

$var = foo();
if (!$var) {
  //...
}

Символы даже экономятся в данном случае, а читаемость, имхо, выше. Особенно если не просто вызов foo(), а какое-то сложное, или просто громоздкое, вроде SomeStaticClass::someStaticMethod(), выражение.

Fedcomp Apr 9 2012 at 17:01

<зануда>Перенос строки тот же символ</зануда>

Fedcomp Apr 9 2012 at 17:02

хотя не заметил, 'alse == ' экономится, прошу прощения.

mr_MAIL Apr 9 2012 at 17:12

Согласен, но читаемость достаточно субъективна, а я хочу показать что вариант с присваиванием в условии имеет право на существование.

faiwer Apr 9 2012 at 17:27

Очень субъективно. На мой взгляд, в вашем читаемость ниже. В моём коде можно часто встретить что-то подобное:

if( !$some = some() )
{
    ...
}
if( ( $some = some() ) === false )
{
   ...
}

Ни у меня, ни у моих коллег подобный код не вызывал проблем с понимаем или скоростью восприятия. Возможно, причина кроется в стиле кода (очень много строк и мало кода). А вот проблемы с читаемостью старых килотонн кода, которые почти ничего не делают, но скролл уже «трещит по швам», возникают часто.

P.S. Всё очень субъективно. Полагаю опытные программисты со стажем 10+ лет пишут такое, что у меня глаза будут разбегаться.

Infthi Apr 9 2012 at 17:35

while(!(flag = foo()) {
bar(flag);
}

порой компактнее получается, чем присвоение как перед циклом, так и внутри его.

VolCh Apr 9 2012 at 19:16

Циклы — отдельный вопрос, специально их не касался.

VolCh Apr 9 2012 at 19:20

for($flag = foo(); !$flag; $flag = foo()) {
  bar($flag);
}

Не будет ли более читаемым, несмотря на большую громоздкость?

petropavel Apr 9 2012 at 20:42

не будет. То есть, не обязательно будет.
Тут дело не в количестве строк или символов, а в количестве юнитов, концепций, логических блоков — не могу слова подобрать.

В варианте с while их меньше, но они сложнее. В варианте с for их больше, но они проще. Скорость восприятия/чтения падает и от количества юнитов и от их сложности.

Мне лично юниты в этом while читать так же так же легко и быстро как и в for, так что for читается медленее (юнитов больше).

Для новичков, вполне могу представить, что условие в while прочитать может быть заметно сложнее, чем в for — и тогда while будет читаться хуже, хотя юнитов там и меньше.

kuzemchik Apr 9 2012 at 17:16

попробуйте посмотреть на это критично, и в перспективе поддержки

if ( false = ($var = foo())) {

 echo $var;
  //...
}

Потом что-то поменялось в логике

if ( true = ($var = foo())) {
 echo $var;
  //...
}

а потом оказалось что теперь надо так

if ( false = $otherVar = foo2()  || true = ($var = foo())) {
 echo $var;
  //...
}

И теперь получается что условие сломало логику внутри так как переменная не изменила значение так как if провалился по первому условию

kuzemchik Apr 9 2012 at 17:18

Верный последний вариант:

if ( false == ($otherVar = foo2())  || true = ($var = foo())) {
 echo $var;
  //...
}

maddogg Apr 9 2012 at 18:01

в плане поддержки — условие выносится вообще в отдельный метод/функцию

Quiz Apr 9 2012 at 17:20

И всё равно «читабельность» не ахти какая. И момент довольно спорный.
И ещё момент — если переменная будет использоваться только внутри условия — что мешает сделать объявление переменной именно внутри этого условия, а не в condition?
Каждый пишет код как привык — кто-то обожает тернарные операции, кто-то пишет if, кто-то switch, а кто-то goto.
Семантически, разумеется, каждый из этих вариантов валиден и нормально отработает. Но как этобудет выглядеть — сугубо эстетический вопрос.

morr Apr 9 2012 at 17:26

Допускаю присваивание внутри if при двух условиях:
1. переменная используется исключительно внутри блока if
2. присваивание происходит в начале условия, а не где-то в середине
if (($var = foo()) == ...)
Во всех остальных случаях согласен, это абсолютное зло.

shmaltorhbooks Apr 9 2012 at 17:27

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

RuslanZavackiy Apr 9 2012 at 17:30

Нужно помнить, что пишете код вы не для себя. И в большинстве случаев, после вас с вашим же кодом, может столкнуться другой программист. И поверьте, будет куда проще разобраться в том, что писали вы, если вы просто сделаете 2 строчки, вместо такого «крутого» выкрутаса.

$var = foo();
if ($var == true)…

faiwer Apr 9 2012 at 17:37

Скажите честно, конкретно в PHP и JavaScript-е, вам такая конструкция:

if( $var == true )
{
}

яснее, нежели такая:

if( $var )
{
}

? Лично мне вторая ближе. Мозг схватывает смысл на лету, а не останавливается на ==, пытаясь «пропарсить» что у нас тут = или == или ===, а потом на true. Ваш пример я бы заменил на:

if( $var = foo() )
{
}

В этом случае я был бы гораздо ближе к пониманию логики сего кода. Я, ни в коем случае, не пытаюсь доказать, что мой подход лучше. Я лишь пытаюсь обратить внимание на то, что всё очень субъективно. И попытка угодить код тем, кто «не для себя», должна быть переосмыслена. Например дискуссией с коллегами :)

RuslanZavackiy Apr 9 2012 at 17:45

На счет этой конструкции — if ($var == true) я погорячился. Сам никогда так не делаю.
Имелось ввиду if ($var) {} или if (func()) {}. Просто в действительности, это упростит поддержку кода в будущем. Если вы точно уверены, что это место не будет дорабатываться, то на этом можно остановиться и любая конструкция сгодится. Но Если вам вдруго понадобится к вашему коду вернуться через год-два, вы будете в крайнем шоке от того, как и что там работает ) редко когда можно помнить весь свой код досканально (если не работать все время над одним проектом).

Я за вынесения присвоения за рамки if/else и других statement'ов. Всетаки использовать надо все операторы по назначению. Если они были думаны для обработки условия, то оно должно быть максимально простым и читабельным. Вы всетаки не спортивным программированием занимаетесь у себя в проекте? )

faiwer Apr 9 2012 at 18:05

>> Если вам вдруго понадобится к вашему коду вернуться через год-два, вы будете в крайнем шоке от того, как и что там работает

Не был. Я свой код забываю на следующий день, поэтому у меня много комментариев и я стараюсь его делать очень простым и понятным. Но моё понимание «простой код» не включается в себя разбитие 1 строки в 5. Я исхожу из такого понимания — что есть «читабельность» кода:

1. понимание 1 строчки
2. понимание блока кода
3. понимание метода

Можно добиться предельной ясности в каждой строке и при этом добиться такого хаоса в 5, что без комментариев чёрт ногу сломит. Вынос каждого действия в отдельную строку (из моего опыта напоминает Delphi, кстати) заставляет писать или очень длинные функции, или функции с очень длинными сложно-читаемыми именами. Должен быть баланс. Я полагаю, что чем больше опыта, тем больший вес имеет пункт 3(4,5?), не даром же в прошлом хабра-холиваре было так много сторонников минимального комментирования кода (мол с опытом он только мешает).

>> Вы всетаки не спортивным программированием занимаетесь у себя в проекте? )

Cколько помню олимпиадки, там сплошные антипатерны. Переменные вида int a,b,c,count, код в одной сплошной куче и полное «забитие» на всё после заветного «Accepted» :)

RuslanZavackiy Apr 9 2012 at 18:09

Вообще, все что говорите — верно. В принципе, я тоже не вижу сильной проблемы в коде:

if ($var = func()) {}

главное чтобы это не стало что-то вроде такого:

if ($var = func() || $test == func2() && $test = func3()) {}

В такой жести уже посложнее разобраться будет )

faiwer Apr 9 2012 at 18:15

Тут есть два нюанса. Первый — я прямо скобко-ман какой-то, а посему бы вынес каждый $var = funcX в отдельные скобки. Второй нюанс заключается в том, что 3 таких операнда уже перебор. Вот два, вполне:

if( ( $var = func() ) || ( $test = func() )
{
    kill_all_...()
}

В вашем примере это не столь очевидно, но чаще всего имена методов длиннее, и конструкции вида $comment->checkBotDefence( true, $comment_item ) уже не позволяют разойтись :)

VolCh Apr 9 2012 at 20:59

if ($var = func()) {}

На таком коде, имхо, глаз споткнётся если вдруг после первого запуска будет логический баг. Первая мысль при анализе кода на место возможного бага: «а действительно ли я хотел сделать тут присваивание, может проверить равенство?», даже если код был написан минуту назад, не говоря о человеке который пытается разобраться в этом коде, который он первый раз видит. Чтобы этого не было нужно или комментировать такое место // да, тут должно быть присваивание, чтобы сэкономить строчку кода, или строчку кода не экономить :)

Imenem Apr 9 2012 at 22:12

И первый и второй варианты вызывают приведение типа, а значит не являются абсолютно прозрачными. Классический пример:

if ($position = strpos('123', '1'))
{
    print_r('Символ найден на позиции ' . $position);
    print "\n";
}
else
{
    print_r('Символ не найден');
    print "\n";
}

imenem@localhost:~
$ test
Символ не найден

Вы можете парировать фразой «я всегда знаю, как результат выполнения функции приводится к boolean», но я считаю, что единственно правильной является строгая проверка, без приведения типа, если уж вы используете такое объявление переменной.

tnz Apr 10 2012 at 10:25

strpos возвращает boolean false, если ничего не нашла. А может вернуть 0, который в условии приведется к false. А 0 будет значить позицию вхождения в данном случае.

if ( false !== ($position = strpos('123', '1')) )
{
    print_r('Символ найден на позиции ' . $position);
    print "\n";
}
else
{
    print_r('Символ не найден');
    print "\n";
}

Imenem Apr 10 2012 at 10:35

А я о чем по вашему написал?

tnz Apr 10 2012 at 10:43

Пардон, был не прав. Ну и вообще, написал более очевидно.

vitalets Apr 9 2012 at 17:32

Мне от трех подряд закрывающих скобки уже не по себе)
По мне так каждая строка кода должна выполнять свою операцию: вызов foo() отдельная строка, проверка условия — тоже отдельная строка.

Infthi Apr 9 2012 at 17:36

нужна лиспотерапия ^^

mentatxx Apr 9 2012 at 17:56

Мне нравится подход XCode'a — принята конвенция, что для присваивания в условии нужно использовать дополнительные скобки.
Тем самым можно и делать красивые конструкции вида

while( (row=[db nextRow]) ) { ... };

и две скобки привлекают внимание.

Случай «забыть одно = в сравнении» также учтен — компилятор предупредит, если мы забылись и написали просто

while( value = [obj yetAnotherMethod] ) { ... };

то увидим предупреждение компилятора

romy4 Apr 9 2012 at 18:07

if ( a )
{
    b -= 5;
    if ( b>0 )
    {
          c = foo();
          if ( c )
              do_something();
    }
}

if ( a )
{
    if ( (b -= 5) > 0 )
    {
          if ( c = foo() )
              do_something();
    }
}

if ( a && (b -= 5) > 0 && (c = foo()) )
    do_something();

Мне с моим стажем как-бэ пофигу какой код отлаживать если он читабелен. Просто новичкам я бы разрешил писать только (1) вариант, пока не проникнуться пониманием записи правильных последовательностей в варианте (3). Первый вариант приводит к стилю arrow-code, что на больших вложениях, больше 2, на читабельность так же влияет негативно.

Mithgol Apr 9 2012 at 23:19

Гм, а если какое-то из первых двух условий false, то до присваивания вообще не дойдёт? Это нормально?

romy4 Apr 9 2012 at 23:25

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

sectus Apr 9 2012 at 18:08

В коротких методах пишу

 if ($var = foo()){...}

это экономит 20%, 25% от общего количество строк метода, если в нём 5, 4 строчки.: )

В большом ветвлении предпочитаю писать так же.

if ($var = foo()){
    }
elseif ($var = foo2()){
    }
elseif ($var = foo3()){
    }
else
    $var = foo4();

Это лучше выглядит, чем вот это:

$var = foo();
if ($var){
    }
else{
    $var = foo2();
    if ($var){
        }
    else{
        $var = foo3();
        if ($var){
            }
        else
            $var = foo4();
    }
}

На самом деле так писать пришлось однажды.

Первый пример пишу вот так:

 if (!($var = foo())){...}

faiwer Apr 9 2012 at 18:18

По поводу последнего пример, а почему не так:

if( !$var = foo() ){ ... }

Я очень активно эксплуатирую такую возможность. Гореть мне в аду или ничего? :)

sectus Apr 9 2012 at 18:22

Да не, всё пучком, просто когда первый раз увидел такую запись, то ничего не понял, испугался и заплакал. Это позже, когда разобрался с приоритетами операторов, я всё понял, но травма осталась… Ж )

kuzemchik Apr 9 2012 at 18:26

$var = true;
switch($var){
	case foo():
            ...
	case foo2():
            ...
	case foo3():
            ...
	default:
            ...
}

Не?

burjui Apr 9 2012 at 18:26

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

Я вообще не понимаю этого фанатизма по поводу числа строк в коде. Это даже не преждевременная оптимизация (скорости компиляции, ага) и не помощь программисту в более быстром чтении кода. Если код короче — это ещё не значит, что его проще понять.

Dlussky Apr 9 2012 at 18:31

Лично я считаю, что данная техника оправдана только в циклах:

while ($data = getNextPieceOfDataOrFalseIfThereIsNothingLeft()) {
    doSomeCoolStuffWith($data);
}

UFO just landed and posted this here

karneds Apr 9 2012 at 20:09

if (file_exists($f = '/modules/' . $class . '/module.php') || file_exists($f ='/class/'. $class . '.php'))
    include $f;

ИМХО, код короче, понять его проще/быстрее и нет копипасты в отличие от

if (file_exists( '/modules/' . $class . '/module.php'))
    include '/modules/' . $class . '/module.php';
elseif(file_exists('/class/'. $class . '.php'))
    include '/class/'. $class . '.php';

qrick Apr 10 2012 at 17:56

а ничего, что во втором варианте подключаются два файла, а в первом лишь

'/class/'. $class . '.php'

karneds Apr 10 2012 at 18:12

Смотрите внимательнее.
Во втором варианте, подключится лишь 1 файл, т.к. стоит elseif, а не if.
Первый случай отработает аналогично, можете проверить:

if($f = true || $f = 1)
    var_dump($f);

bool(true)

if($f = 0 || $f = true)
    var_dump($f);

bool(true)

weirdan Apr 9 2012 at 20:56

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

Paulus Apr 10 2012 at 02:48

Делается это именно для того, чтобы избежать случайного присваивания переменно $var значения

И что тут еще может быть непонятно????

faiwer Apr 10 2012 at 10:54

И часто у вас бывают такие ошибки?

Paulus Apr 12 2012 at 03:11

Никогда. Ибо я давно знаю то, что безуспешно пытаются вам объяснить ваши коллеги ;)

maximw Apr 10 2012 at 12:25

Имхо, лучше не мешать вместе логику присваивания и логику сравнения.

Выше уже был показан пример, когда при изменении логики сравнения рушилась логика присваивания (второй операнд оператора || не просчитывается если первый операнд true).

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