Плохое форматирование и непонятный алгоритм функции генерации идентификаторов


Исходный код

function id($code){
$id=@m_q("select id from `id`")+1;
@lng();
mysql_query("UPDATE `id` SET `id` = '$id', `odin` = '1' WHERE odin = '1'");
$id="$id";$r="$id";$r_k=strlen($r);
$m="";
$m_k=strlen($m);$s=$m_k-$r_k;
$t=substr($m,0,$s);$id="$t$id";
$old_id="$id";
if(isset($code)):$id=md5($id);
$id=substr($id,0,18);$id="$id";
endif;return$id;
}

Что не так в исходном коде

Плохое форматирование

Отсутствует отступ строк.

Отступ измеряется уровнями, один уровень в PHP обычно 4 позиции.

Если объявление функции function id начинается на нулевом уровне (без отступа), то все строки внутри функции должны быть на первом уровне отступа, то есть начинаться с позиции 4. Закрывающиая фигурная скобка функции должна быть снова без отступа.

Строки внутри прочих конструкций идут с увеличенным отступом. Таким образом, строки внутри оператора if(isset($code)) должны начинаться на позиции 8.

Несколько операторов на одной строке.

Такой вариант форматирования кода сильно снижает его читаемость. Глобальное правило хорошего кода: одна строка — один оператор. Операторы в этих строках необходимо разъединить:

$id="$id";$r="$id";$r_k=strlen($r);
$m_k=strlen($m);$s=$m_k-$r_k;
$t=substr($m,0,$s);$id="$t$id";
if(isset($code)):$id=md5($id);
$id=substr($id,0,18);$id="$id";
endif;return$id;

Больше воздуха.

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

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

Не говорящие названия переменных

Названия всех переменных, кроме $id и $old_id, совершенно ничего не говорят о своём смысле и предназначении.

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

Мёртвый код

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

Например, вот этот фрагмент ничего не делает, так как переменной $m изначально присваивается пустая строка:

$id="$id";$r="$id";$r_k=strlen($r);
$m="";
$m_k=strlen($m);$s=$m_k-$r_k;
$t=substr($m,0,$s);$id="$t$id";

Это присвоение не нужно, так как переменная $old_id нигде не используется:

$old_id="$id";

Переменная $r тоже фактически не нужна, так как символьную длину идентификатора можно получить непосредственно из переменной $id, без участия $r.

Рискованный алгоритм генерации идентификаторов

При параллельном выполнении (что не редкость для веб скриптов) код

$id=@m_q("select id from `id`")+1;
mysql_query("UPDATE `id` SET `id` = '$id', `odin` = '1' WHERE odin = '1'");

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

Для гарантии уникальности следует пользоваться другими алгоритмами, например транзакциями или полем с нативным автоинкрементом твоей СУБД.