Две раздражающие частности в 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 в массиве появились пропуски. В обычный массив такое в джейсоне превратить нельзя, поэтому ПХП сделал из него объект с числовыми ключами. Гоу такое не простил — у него статическая типизация и в него нельзя пихать всякое.
Микросервис выдал ошибку в лог, данные не принял. У тестеров возникла очень странная ошибка, которая повторялась в каких-то очень редких условиях. Хорошо, что логи были.
Увы, но документацию к языкам сейчас никто подробно не читает, а если читает, то такие особенности не помнит — такова данность и ничего не изменить. Хотелось бы чтобы в языках было меньше такого поведения, либо чтобы специализированные редакторы как-то обращали внимание программиста на такие мелочи.
Про второй момент быстро запоминаешь, когда работаешь с монгой, например.
Я себе ввожу в привычку array_keys(array_flip(...)).
Насколько я помнил, flip ещё и быстрее unique. Провёл замеры, массив 400 тысяч элементов, каждое значение попадается дважды, сто итераций, PHP 7.1.14:
array_keys(array_flip(…)) — 1,6 секунды
array_values(array_unique(…)) — 68 секунд
По этой причине и начал использовать такой подход. Но в реальных проектах это почти не играет роли — очень редко в массиве бывает больше пары сотен элементов.
У нас как-то раз было 700+ тысяч, пришлось оптмизироваться ещё и по памяти — например, array_map заменить в одном месте на array_walk.
Бывает, но это редкость. В таких ситуациях, обычно, выгоднее поменять алгоритм формирования массива, вместо постобработки.
Юнит-тесты для этого есть.
Кто-то добавит array_unique, значит держал в голове вариант, что данные могут быть не уникальны. В голове держал, но не протестировал.
vasa_c, а как тут помогут тесты? На выходе все норм, данные уникальны. Ломаются только ключи.
@aktuba
$actual = array_unique([1, 1, 2]);
$expected = [1, 2];
$this->assertEquals($expected, $actual); // Failed asserting that two arrays are equal
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)
норм? или нет? а ведь массивы уже и не уникальны вовсе ;)
тесты надо писать правильно, иначе в них нет смысла, о чем и было сказано в моем комментарии выше.
@aktuba я не тестировал функцию array_unique, я просто привёл минимальный пример.
ваш метод должен возвращать упорядоченный массив — простой assertEquals() отловит, если это не так.
вы использовали зачем-то в реализации этого метода array_unique — значит вы думали о том, что значения на входе могут быть не уникальны — протестируйте подобный вход
не надо демагогии, про пишите тесты. у меня они отлавливают огромное количество вот таких вот мелочей, когда мне было лень досконально документацию по всем используемым функциям прочитать.
Если не знать таких мелочей, то какие тесты отловят ошибки в тестах? Или вы их не на ПХП пишете?
Кстати это только выглядит просто — ну не мог же программист не знать, что тут не уникальные значения, раз array_unique поставил. Но в жизни там модели, база и всё такое, когда значения не уникальны — редкий случай, не так-то просто сунуть в модели такие данные, чтобы на выходе получить не уникальность. А без этого это превращается в тест интерпретатора, так можно дойти до того, что и сложение будем проверять.
$this->assertEquals([0, 2], [null, ’2’]);
точно выловит?)
верно. и как показано выше, assertEquals сработает не верно. По сути, assertEquals — это ’==’, что категорически не приемлимо для сравнения массовов.
не поверите — у меня тоже. именно поэтому и знаю, что протестировать метод, который возвращает уникальные значения массива (даже плоского, не говоря про несколько уровней) не так просто. именно это я сказал выше.
еще вопрос напоследок: если надо протестировать уникальные значения, почему вы тестируете еще и индексы?)
Я всё ещё не понимаю проблемы. На выходе должен быть порядковый массив, а он не порядковый. Любая попытка сравнить полученный результат с ожидаемым должна это выявить.
По поводу array_unique — может помочь статический анализатор. В большинстве случаев он умеет подсказывать о странном поведении функций.
@vasa_c:
Откуда взялось это утверждение?
@thefish:
Тут нет «странного поведения». Оно задокументированное + часто используется именно в таком виде: оставить уникальные значения с ключем из первого попадания.
Э-э... из текста этой статьи, разве не в этом вообще вся соль этого разговора?
Ну давайте ещё мильён примеров приведём. Стопроцентную гарантию даёт только страховой полис. А тесты дают 90% выловленных глупых ошибок. От того, что никогда нельзя отловить 100, нам что, забить и не ловить эти 90?
Где надо, почему? Предполагается, что на выходе порядковый массив, это и протестировано.
Где подобное в статье? Нет, соль в другом)
Эмм... Так это вы утверждали, что тест выловить ошибку. Я дал пару примеров, что это не так.
Понятно. Вы любите подгонять задачи под себя. Не увидел ни намека в посте на порядок. Замените массив в посте на [1 => 1, 5 => 5, 6 => 5, 12 => 6]. От этого суть поста не поменяется.
aktuba, простите, мне не кажется, что вам интересно в этом разговоре хоть что-то, кроме самолюбования.
))) ну ок. Но вы не правы)
aktuba, возможно и не прав, а, возможно, не смог донести свою мысль. просто пишите тесты и не заморачивайтесь.
так разве-ж я хоть раз высказался против тестов?))) наоборот — тесты это гуд, но в данном случае, если написать их быстро и не обдумано, они не помогут.
видимо, мы просто по разному смотрим на пост. для меня это ежедневная проблема, т. к. я каждый день делаю код-ревью и подобные моменты попадаются — люди пишут тесты не подумав, что и как надо проверить. в случае с array_unique — вот сценарий, когда и с тестами норм, и с реализацией вроде норм, но упадет примерно как в посте:
и после 5-го шага имеем поломанное апи и нормально проходящие тесты.
знаешь сколько я натыкался на подобные сценарии? овер-много. потому и задевает, что ты пишешь про тесты, на которых я уже ломал и руки, и ноги, и голову)
В Пиэйчпи. Будь последовательным.
Я так не говорю.