Соответствует ли данный код хорошим практикам?

Question

[kosty_bot]

Такие длинные строчки с sql-запросами в скобках цикла/условия/и тп - это нормально?
Опыта ноль, и я понятия не имею как это обычно выглядит, а писать спагетти-код не хочется.

$db = new Database();
$tag_id = $_POST['tag_id'];

if (!$db->query("select id from tags where id = $tag_id")->fetchArray()['id']) {
    exit('Тег не найден');
}

$tag_names = array();
foreach ($db->query("select name from tag_names where tag_id = $tag_id")->fetchAll() as $name) {
    array_push($tag_names, $name['name']);
}

Comments

[Vlad]

а если будет 1; DROP TABLE tags; -- в $_POST['tag_id']
это норм?
на хабре есть статейка на эту тему

[FanatPHP]

в первую очередь надо выкинуть на помойку класс Database

Answer 1

[Сергей Горностаев]

Вам больше стоит обеспокоиться тем, что вы запросы формируете интерполяцией строк.

Comments

[Сергей Горностаев]

Agent Smith, https://en.wikipedia.org/wiki/String_interpolation

Answer 2

[Vlad]

можно одним запросом, типо такого:

$db = new Database();
$tag_id = $_POST['tag_id'];
$stmt = $db->prepare("SELECT `tag_names`.`name` FROM `tags` INNER JOIN `tag_names` ON `tags`.`id` = `tag_names`.`tag_id` WHERE `tags`.`id` = ?");
$stmt->execute([$tag_id]);
$rows = $stmt->fetchAll(PDO::FETCH_ASSOC);

if (count($rows)<1) {
    exit('Тег не найден');
}

$tag_names = array();
foreach ($rows as $row) {
    array_push($tag_names, $row['name']);
}

помните про sql инъекции!
upd. поменял на более безопасный код

Comments

[Dmitry Bay]

отмазался

[Vlad]

Dmitry Bay, вы по поводу sql инъекций? не вопрос, просто лучше прочитать про это и понять как исправить, чем слепо скопировать готовое.
upd. (скопировал в ответ)

$db = new Database();
$tag_id = $_POST['tag_id'];
$stmt = $db->prepare("SELECT `tag_names`.`name` FROM `tags` INNER JOIN `tag_names` ON `tags`.`id` = `tag_names`.`tag_id` WHERE `tags`.`id` = ?");
$stmt->execute([$tag_id]);
$rows = $stmt->fetchAll(PDO::FETCH_ASSOC);

if (count($rows)<1) {
    exit('Тег не найден');
}

$tag_names = array();
foreach ($rows as $row) {
    array_push($tag_names, $row['name']);
}

[FanatPHP]

только перепишите на рабочий вариант

$stmt = $db->prepare("SELECT `tag_names`.`name` FROM `tags` INNER JOIN `tag_names` ON `tags`.`id` = `tag_names`.`tag_id` WHERE `tags`.`id` = ?");
$stmt->execute([$tag_id]);
$rows = $stmt->fetchAll(PDO::FETCH_ASSOC);

[FanatPHP]

а. вот ниже его уже написали. и запрос там заодно не такой адовый

[Vlad]

FanatPHP, точно, спасибо ))
К сожалению знаний не хватает чтобы так сокращать запрос

[FanatPHP]

ну и вам тоже порекомендую освоить PDO

Answer 3

[mathrep]

первое

Comments

[ThunderCat]

И компот!

Answer 4

[Андрей Рудаков]

Да верно, по возможности не используйте запросы в цикле.