Могу ли я получить отзыв об этой функции PHP, которая проверяет, зарегистрировался ли пользователь?

Question

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

function test_user($user) {
$conn = get_db_conn();
$res = mysql_query("SELECT * FROM users WHERE uid = $user");
$row = mysql_fetch_assoc($res);
if (count($row) == 1) {
return true;
}
else {
    return false;
}
}

Когда кто-то входит в систему, у меня есть его UID. Я хочу посмотреть, если это уже в БД. Это базовая логика будет использоваться в

«Если существует, отобразить настройки, если! Существует, отобразить поле регистрации». Очевидно, что это зависит от того, как он используется в остальной части кода, но будет ли это работать так, как рекламируется, и попал ли я в ловушку? Спасибо!

Answer 1

Попробуйте это:

$conn = get_db_conn(); # should reuse a connection if it exists

# Have MySQL count the rows, instead of fetching a list (also prevent injection)
$res = mysql_query(sprintf("SELECT COUNT(*) FROM users WHERE uid=%d", $user));

# if the query fails
if (!$res) return false;

# explode the result
list($count) = mysql_fetch_row($res);
return ($count === '1');

Мысль:

• Вам потребуется лучшая обработка неудавшегося запроса, поскольку возвращение false означает, что пользователь еще не существует.

• Используйте базу данных для подсчета, это будет быстрее.

• Я предполагаю, что uid - это целое число в выражении sprintf. Теперь это безопасно для ввода пользователем.

• Если у вас есть оператор if, который выглядит как if (something) { true } else { false }, вы должны свернуть его до return something.

НТН

Answer 2

Кроме того,

if (condition) {
    return true;
}
else {
    return false;
}

можно переписать как:

return condition;

, что значительно экономит время при наборе текста и чтении:)

Answer 3

Отступ! В целом это выглядит неплохо ... проверьте комментарии ..

function test_user($user)
{
   $conn = get_db_conn(); //this should be done only once. Maybe somewhere else...?
   $res = mysql_query("SELECT uid FROM users WHERE uid = $user");
   $row = mysql_fetch_assoc($res);
     //I can't remember...can you return count($row) and have that forced to boolean ala C? It would reduce lines of code and make it easier to read.
   if (count($row) == 1) {
      return true;
   }
   else {
      return false;
   }
}

Answer 4

Прежде всего, вам нужно позвонить

$user = mysql_real_escape_string($user);

, поскольку в вашем коде есть ошибка SQL-инъекции, см. Руководство . Во-вторых, вы можете упростить свою логику, изменив запрос на:

SELECT COUNT(1) FROM user WHERE uid = $user;

, которая просто позволяет вам оценить одно возвращаемое значение из $row. И последнее, как только вы освоите основы php, подумайте о том, чтобы взглянуть на php framework. Они могут доставить вам неприятности и не заставят вас писать хороший код, но, скорее всего, сэкономят вам много работы.

Answer 5

Это многократно, да. Возможно, вы захотите удалить SQL из самого кода PHP.

Хотя вы не обязательно запрашивали оптимизацию, возможно, вы захотите рассмотреть вопрос о настройках отображения пользователя (которые, как я предполагаю, хранятся в БД) и, если он вернется пустым, отобразить поле регистрации. Вы сохраните поездку в базу данных, и в зависимости от вашего трафика, это может быть огромным. Если вы решите сохранить эту реализацию, я бы предложил выбрать только один столбец из базы данных в вашем SELECT. Пока вы не заботитесь о данных, нет причин извлекать каждый столбец.