Исходный код
function PagerStarter( $object_id, $object_url, $dbt, $maxwidth, $edittime, $level_step, $comments_per_page )
{
if ( $_GET['from'] ) { $from = $_GET['from']; }
if ( $_POST['from'] ) { $from = $_POST['from']; }
if ( $_GET['current_page'] ) { $current_page = $_GET['current_page']; }
if ( $_POST['current_page'] ) { $current_page = $_POST['current_page']; }
if ( ( empty( $current_page ) == TRUE ) && ( empty( $from ) == TRUE ) )
{
$r = mysql_query( "SELECT * FROM ".$dbt." WHERE parent = '1' AND parent_image = '$object_id'".$e."" );
$r_count = mysql_num_rows ( $r );
$total_pages = 0;
$i = 0;
while ( $i < $r_count )
{
$total_pages = $total_pages + 1;
$i = $i + $comments_per_page;
}
if ( $total_pages == 0 )
{
$total_pages = 1;
}
for ( $i = 1 ; $i <= $total_pages ; $i++ )
{
$from_q = ( $i * $comments_per_page ) - $comments_per_page + 0;
}
$_GET['current_page'] = $total_pages;
$_GET['from'] = $from_q;
}
}
Что не так в этом коде
Изменение параметров скрипта
$_GET['current_page'] = $total_pages;
$_GET['from'] = $from_q;
Это плохой способ вернуть результат выполнения функции, потому что затрудняет отслеживание изменений в данных.
В какой-то момент ты обнаруживаешь, что передал в скрипт одни параметры, а в обработку пошли какие-то совсем другие, и невозможно найти место, где произошло превращение.
Запомни: $_GET
и $_POST
— это входные параметры скрипта в точности в том виде, как они пришли в запросе, в любой точке скрипта. Они всегда отправная точка для твоего алгоритма, а вовсе не объект обработки. Любые вычисления на основе этих параметров нужно складывать в новые переменные. Точка.
Как нужно:
return [ $total_pages, $from_q ];
}
Избыточные параметры
function PagerStarter( $object_id, $object_url, $dbt, $maxwidth, $edittime, $level_step, $comments_per_page )
Из семи параметров четыре не используются в коде функции. К тому же, семь параметров — это априори слишком много. Скорее всего, функцию с семью параметрами нужно разбить как минимум на две поменьше, потому что у такой функции больше одной ответственности.
Не говорящее название функции
function PagerStarter(
Название функции не отражает того, на что она нацелена — вычислить номер страницы на основе входных параметров либо по умолчанию.
Более подходящее имя: GetCurrentPage
.
Сравнение булевых величин
Нет смысла сравнивать результат операции empty
с булевым литералом, да ещё и нестрогим сравнением. empty
и так возвращает булево значение, его можно и нужно непосредственно использовать как аргумент логического выражения:
if ( empty( $current_page ) && empty( $from ) )
Подсчёт строк через mysql_num_rows
Нельзя подсчитывать количество строк через mysql_num_rows
, если ты не собираешься их все использовать. В данном примере использование строк отсутствует, поэтому необходимо воспользоваться запросом SELECT COUNT
. Если этого не сделать, все строки, подпадающие под условие, будут вытащены в память скрипты, что может обернуться нехорошими последствиями (перерасход памяти и тормоза).
$r = mysql_query( "SELECT COUNT(*) FROM ".$dbt." WHERE parent = '1' AND parent_image = '$object_id'".$e."" );
$r_count = mysql_fetch_array( $r )[ 0 ];
Данное решение не является преждевремнной оптимизацией, потому что первоначальный вариант является априори неправильным решением задачи.
Арифметические вычисления через цикл
$i = 0;
while ( $i < $r_count )
{
$total_pages = $total_pages + 1;
$i = $i + $comments_per_page;
}
if ( $total_pages == 0 )
{
$total_pages = 1;
}
for ( $i = 1 ; $i <= $total_pages ; $i++ )
{
$from_q = ( $i * $comments_per_page ) - $comments_per_page + 0;
}
В данном фрагменте кода выполняется расчёт двух величин: количество страниц пагинации в списке элементов и смещение для последней страницы.
Причём второй цикл в принципе ничего не делает, так как на каждой следующей итерации затирает результат предыдущей.
По сути для их расчёта не требуется ничего сложнее элементарной школьной арифметики:
$total_pages = ceil($r_count / $comments_per_page);
if ( $total_pages == 0 )
{
$total_pages = 1;
}
$from_q = ($total_pages - 1) * $comments_per_page;
Теория
- Названия функций
- Ребусы в коде