Одна из важных техник, которую в программировании необходимо осваивать с самого начала - это декомпозиция, разбиение задачи на подзадачи. Обычно задача разбивается не на самые мелкие подзадачи, а на функциональные блоки, которые, в свою очередь, также могут разбиваться, и так хоть до бесконечности.
Декомпозиция позволяет выделить эти функциональные блоки, обернуть их в функции и вынести из основного кода. Именно так получается, что мы работаем со строками и не вынуждены напрямую дергать процессор, чтобы получить какое-то значение (если только не пишем на ассемблере) - мы работаем со строковыми функциями.
Конкретно вашу задачу можно разбить так:
- Поиск пользователя
- Валидация поисковой строки
- Проверка на пустоту
- Проверка на слишком маленькую длину
- Проверка на слишком большую длину
- Поиск в базе данных
- Подключение к базе данных
- Выполнение запроса
- Получение результата
- Закрытие подключения
- Вывод результата
- Вывод лейаута (общего стиля сайта)
- Вывод шаблона страницы поиска
- Вывод данных или сообщения валидации
Все это (почти все) сейчас реализуется в одной функции, что увеличивает связность кода, препятствует рефакторингу и еще много умных слов - все это в результате дает а) необходимость править функцию поиска при любой правке шаблона / запроса поиска / правил валидации, б) уменьшение концентрации (я не шучу) - при работе с этой функцией приходится отвлекаться на те вещи, которые не имеют отношения к текущим правкам, в) потерю времени, когда таких функций становится много, и поиск местанахождения того или иного куска кода становится проблемой, г) проблемы с отладкой из-за недостаточной изолированности (предположим, что mysql багнулась и не выполняет почему-то запрос - перейти на другой движок будет сложно).
Что будет, если разбить эту большую функцию на много мелких? Она будет выглядеть примерно вот так:
function actionSearch($query)
{
// Пускай validate_search_query возвращает true, если все окей, и массив ошибок, если окей не все
$validationResult = validate_search_query($query);
if ($validationResult !== true) {
$data = array('errors' => $validationResult);
} else {
// Пускай возвращает или массив ['errors' => []] или ['users' => []]
$data = find_users_by_query($query);
}
// Запускаем рендеринг шаблона с полученными данными, будь то ошибки или нужные данные. Шаблон уже сам разберется об удачности или неудаче поиска по наличию элемента 'errors' в массиве.
render('search', $data);
}
(на самом деле в процедурном программировании сведения об ошибках будут доставаться немного иначе, но для примера сойдет)
Если привести функцию к такому виду, то при наличии проблем с поиском можно будет копать не ее всю, а непосредственно find_users_by_query()
, смотреть на ее вывод и не задумываться ни о че остальном.
Теперь что до существующей функции:
$row = mysql_fetch_assoc($result);
$num = mysql_num_rows($result);
...
do {
...
} while ($row = mysql_fetch_assoc($result));
это очень жесткий антипаттерн. Во-первых, человек, глядящий на do .. while
не понимает, как цикл выполняется в первый раз (ведь $row
появится только тогда, когда цикл дойдет до while
), во-вторых, никто не гарантирует, что мускуль вернул данные вообще, и
$row = mysql_fetch_assoc($result);
имеет полное право не сработать, если нашлось ноль строк.
Реальная же проблема, насколько понимаю, кроется в этой строчке:
if (mysql_affected_rows() > 0)
affected rows - это количество измененных строк, т.е. количество добавленных, обновленных или удаленных строк; в этом запросе таких нет, поэтому результат этой функции будет продолжать возвращать вам ноль, и условия не будет выполняться. Вам нужна mysql_num_rows()
, которая зачем-то лежимт чуть ниже.
Эта строка:
$q1 = "SELECT `id` FROM `users` WHERE `id` = '$row[page_id]'";
не имеет никакого смысла, так как выбирает, по факту, $row['page_id']
. Это имело бы смысл для проверки существования пользователя, но т.к. эти данные получены от движка БД, он уже гарантированно существует.
Если переписывать критическую часть функции, то она будет выглядеть так:
$result = mysql_query($q);
$numRows = mysql_num_rows($result);
if ($numRows > 0) {
$text = "<p>По запросу <b>$query</b> найдено $numRows совпадений.</p>";
while ($row = mysql_fetch_assoc($result)) {
$text .= '<p>';
$text .= '<a href="/users.php?id='.$row['id'].'">';
$text .= $row['name'].' '.$row['surname'];
$text .= '</a>';
$text .= '</p>';
}
} else {
$text = '<p>По вашему запросу ничего не найдено.</p>';
}
p.s. c индентацией в этот раз гораздо лучше