Две раздражающие частности в PHP

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

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

Первый относится к функции exec. Свой второй параметр она получает по ссылке и выводит туда массив строк, которые выдаёт запускаемая команда. Беда возникает в следующем коде:

exec('1st command', $out, $ret);
// … проверяем $ret, делаем что-то полезное с $out
exec('2nd command', $out, $ret);
// … проверяем $ret, делаем что-то полезное с $out

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

Такое поведение описано в руководстве и возможно это полезно для цикла с накоплением, но в общем случае контринтуитивно.

Второй раздражающей частностью является результат работы array_unique. Опять же, особенность описана в руководстве, но про неё нередко забывают: эта функция сохраняет ключи массива. Вот как это выглядит на практике:

var_dump(array_unique([1,1,2]));
/* array(2) {
  [0]=>
  int(1)
  [2]=>
  int(2)
} */

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

Самый замысловатый баг, который я помню был таким. Результирующий массив превращался в джейсон-строку и передавался микросервису на Гоу. Чаще всего значения были уникальными и всё работало как ожидается.

Но однажды звёзды сошлись так, что уникальность нарушилась и в результате работы array_unique в массиве появились пропуски. В обычный массив такое в джейсоне превратить нельзя, поэтому ПХП сделал из него объект с числовыми ключами. Гоу такое не простил — у него статическая типизация и в него нельзя пихать всякое.

Микросервис выдал ошибку в лог, данные не принял. У тестеров возникла очень странная ошибка, которая повторялась в каких-то очень редких условиях. Хорошо, что логи были.

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

Поделиться
Отправить
24 комментария
aktuba

Про второй момент быстро запоминаешь, когда работаешь с монгой, например.

Я себе ввожу в привычку array_keys(array_flip(...)).

Евгений Степанищев

Насколько я помнил, flip ещё и быстрее unique. Провёл замеры, массив 400 тысяч элементов, каждое значение попадается дважды, сто итераций, PHP 7.1.14:

array_keys(array_flip(…)) — 1,6 секунды
array_values(array_unique(…)) — 68 секунд

aktuba

По этой причине и начал использовать такой подход. Но в реальных проектах это почти не играет роли — очень редко в массиве бывает больше пары сотен элементов.

Евгений Степанищев

У нас как-то раз было 700+ тысяч, пришлось оптмизироваться ещё и по памяти — например, array_map заменить в одном месте на array_walk.

aktuba

Бывает, но это редкость. В таких ситуациях, обычно, выгоднее поменять алгоритм формирования массива, вместо постобработки.

vasa_c

Юнит-тесты для этого есть.
Кто-то добавит array_unique, значит держал в голове вариант, что данные могут быть не уникальны. В голове держал, но не протестировал.

aktuba

vasa_c, а как тут помогут тесты? На выходе все норм, данные уникальны. Ломаются только ключи.

vasa_c

@aktuba

$actual = array_unique([1, 1, 2]);
$expected = [1, 2];
$this->assertEquals($expected, $actual); // Failed asserting that two arrays are equal

aktuba

vasa_c, честно, мне лень лезть в дебри, но... ё-моё... что тестирует ваш код? уникальные значения? вроде нет:

$this->assertEquals([2, 1], array_unique([1, 2])); // Failed asserting that two arrays are equal

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

$actual = array_unique([1, 1, 2]);
sort($actual);
$expected = [1, 2];
$this->assertEquals($expected, $actual); // OK (1 test, 1 assertion)

и, о чудо, тесты проходят ;)

В реальности, тест будет еще сложнее, т. к. вот такой тест тоже пройдет, но не должен в большинстве случаев:

$actual = array_unique([1, 1, 2]);
sort($actual);
$expected = [’1’, ’2’];
$this->assertEquals($expected, $actual); // OK (1 test, 1 assertion)

а как насчет такого теста:

$actual = array_unique([0, 2]);
sort($actual);
$expected = [null, ’2’];
$this->assertEquals($expected, $actual); // OK (1 test, 1 assertion)

норм? или нет? а ведь массивы уже и не уникальны вовсе ;)
тесты надо писать правильно, иначе в них нет смысла, о чем и было сказано в моем комментарии выше.

vasa_c

@aktuba я не тестировал функцию array_unique, я просто привёл минимальный пример.

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

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

Евгений Степанищев

Если не знать таких мелочей, то какие тесты отловят ошибки в тестах? Или вы их не на ПХП пишете?

Кстати это только выглядит просто — ну не мог же программист не знать, что тут не уникальные значения, раз array_unique поставил. Но в жизни там модели, база и всё такое, когда значения не уникальны — редкий случай, не так-то просто сунуть в модели такие данные, чтобы на выходе получить не уникальность. А без этого это превращается в тест интерпретатора, так можно дойти до того, что и сложение будем проверять.

aktuba

ваш метод должен возвращать упорядоченный массив — простой assertEquals() отловит, если это не так.

$this->assertEquals([0, 2], [null, ’2’]);

точно выловит?)

вы использовали зачем-то в реализации этого метода array_unique — значит вы думали о том, что значения на входе могут быть не уникальны — протестируйте подобный вход

верно. и как показано выше, assertEquals сработает не верно. По сути, assertEquals — это ’==’, что категорически не приемлимо для сравнения массовов.

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

не поверите — у меня тоже. именно поэтому и знаю, что протестировать метод, который возвращает уникальные значения массива (даже плоского, не говоря про несколько уровней) не так просто. именно это я сказал выше.

еще вопрос напоследок: если надо протестировать уникальные значения, почему вы тестируете еще и индексы?)

vasa_c

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

thefish

По поводу array_unique — может помочь статический анализатор. В большинстве случаев он умеет подсказывать о странном поведении функций.

aktuba

@vasa_c:

На выходе должен быть порядковый массив, а он не порядковый.

Откуда взялось это утверждение?

@thefish:

По поводу array_unique — может помочь статический анализатор. В большинстве случаев он умеет подсказывать о странном поведении функций.

Тут нет «странного поведения». Оно задокументированное + часто используется именно в таком виде: оставить уникальные значения с ключем из первого попадания.

vasa_c

На выходе должен быть порядковый массив, а он не порядковый.

Откуда взялось это утверждение?

Э-э... из текста этой статьи, разве не в этом вообще вся соль этого разговора?

vasa_c

точно выловит?)

Ну давайте ещё мильён примеров приведём. Стопроцентную гарантию даёт только страховой полис. А тесты дают 90% выловленных глупых ошибок. От того, что никогда нельзя отловить 100, нам что, забить и не ловить эти 90?

еще вопрос напоследок: если надо протестировать уникальные значения, почему вы тестируете еще и индексы?)

Где надо, почему? Предполагается, что на выходе порядковый массив, это и протестировано.

aktuba

Э-э... из текста этой статьи, разве не в этом вообще вся соль этого разговора?

Где подобное в статье? Нет, соль в другом)

Ну давайте ещё мильён примеров приведём.

Эмм... Так это вы утверждали, что тест выловить ошибку. Я дал пару примеров, что это не так.

Предполагается, что на выходе порядковый массив, это и протестировано.

Понятно. Вы любите подгонять задачи под себя. Не увидел ни намека в посте на порядок. Замените массив в посте на [1 => 1, 5 => 5, 6 => 5, 12 => 6]. От этого суть поста не поменяется.

vasa_c

aktuba, простите, мне не кажется, что вам интересно в этом разговоре хоть что-то, кроме самолюбования.

aktuba

))) ну ок. Но вы не правы)

vasa_c

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

aktuba

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

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

  1. имеем базу с записями [1 => ’a’, ’2’ => ’b’, 3 =’c’]
  2. реализовываем метод получения уникальных данных из базы, используем array_values. Пишем тесты. Все отлично, все работает, тесты проходят.
  3. добавляем в проект экспорт данных из базы в json. все отлично, все работает, тесты проходят.
  4. добавляем в базу данные [4 => ’a’, 5 => ’b’]
  5. все работает, тесты проходят.
  6. удаляем из базы записи с id 1 и 2. тесты поломались, хотя логика правильная, т. к. на выходе массив с уникальными значениями — поправили тесты, чтобы не проверять индексы.

и после 5-го шага имеем поломанное апи и нормально проходящие тесты.
знаешь сколько я натыкался на подобные сценарии? овер-много. потому и задевает, что ты пишешь про тесты, на которых я уже ломал и руки, и ноги, и голову)

Fyodor Ustinov

В ПХП немало

В Пиэйчпи. Будь последовательным.

Евгений Степанищев

Я так не говорю.

Популярное