Можете что-то подсказать по-поводу кода?

Question

[Simasik]

Вот мой исходный код

public function update($table_name, $pole, $value, $where){
			$pole = explode(", ", $pole);
			$values = explode(", ", $value);
			if(count($pole) == count($values)){

				for($i = 0; $i < count($pole); $i++){
					$que .= $pole[$i]." = ?, ";

				}
				$que = substr($que, 0, -2);
				$STH = $this->connect->prepare("UPDATE ".$this->table_name($table_name)." SET ".$que." WHERE ".htmlspecialchars($where));  
				$STH->execute($values);
			}
			else{
				return false;
			}
		}

Подскажите, как лучше написать этот метод! Расскажите мне об моих ошибках! Мне нужно создать класс для работы с базой данных с драйвером PDO, но методы работают медленно и я не уверен в безопасности кода.

Comments

[FanatPHP]

Чтобы ответ не пропал. Проблема в сайте, на котором ты пытаешься задавать свой вопрос.
Ты думаешь что здесь круглые сутки сидят гуру программирования, не спят, не едят - ждут твоего вопроса, чтобы ответить. Должен тебя огорчить. Гуру программирования на то и гуру, что занимаются более полезными делами. А ошивается здесь всякая шантрапа, которая и рада в ответ на вопрос "как учить язык" дать ссылку на php.net.
Мануал - это справочник. В котором можно посмотреть синтаксис. А программированию на php.net не учат.

Вторая твоя проблема - видеокурсы. Опять же: профессионалам есть чем заняться, и поэтому видеокурсы снимают совсем уж отбросы общества.

По поводу чтения. Не всякое чтение одинаково полезно. Человек должен созреть до той или иной книжки. иначе он из нее ничего не поймет. Отсюда становится понятным, что самое главное - опыт. Надо писать, тестировать написанное в деле, переписывать, снова тестировать - и так далее.

По поводу метода. Для года занятий тяп-ляп в качестве хобби код нормальный. Не нужно думать, что программирование проще медицины. Не проще. На хирурга сколько надо учиться? А ты сколько? Немного терпения, и через 5 лет уже будут у тебя нормальные методы.

По поводу "порвали". Если бы хоть как-то реагировал на критику! Если бы хоть что-то спрашивал, ходил по ссылкам, пробовал. Тебе между прочим дали развернутый ответ. Ты его хоть раз прокомментировал?

Answer 1

[FanatPHP]

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

Основная проблема твоего кода - его чудовищная уязвимость, дыры просто со всех сторон.

Вопросы:
при чем здесь mysqi, если ты пишешь под PDO?
какие методы работают медленно и с чего ты это взял?
зачем тебе нужно создать класс для работы с базой данных с драйвером PDO?

Теперь к ноукам.

Если говорить об удобстве выполнения запроса UPDATE, то идеальным вариантом будет кастомный плейсхолдер для массива с данными + стандартная функция для выполнения запросов.

Пример, с использованием phpfaq.ru/safemysql
Если у нас тупой апдейт

$db->query("UPDATE ?n SET ?u WHERE id = ?i", $table, $data, $id);

обращение к этому методу не сложнее, чем к твоему специальному, но при этом любые сложности здесь решаются с легкостью, а у тебя - никак:

$sql = "UPDATE ?n SET count = count+1, NOW(), ?u WHERE id = ?i";
$db->query($sql, $table, $data, $id);

Если все же хочется именно средствами PDO, то читаем тут phpfaq.ru/pdo
в основном про экранирование имени таблицы и имен полей, и про сборку корректного запроса из пар ключ-значение

Comments

[Simasik]

Как залатать дыры? Что делать? На что ориентироваться?

[FanatPHP]

ответь сначала на вопросы

Answer 2

[Евгений]

Если делаешь свой велик, то можно немного упростить работу с пдо
https://github.com/colshrapnel/thebestpdowrapper
А вообще, зачем велосипед, если есть куча решений?
https://laracasts.com/lessons/how-to-use-eloquent-...

Comments

[FanatPHP]

первый, прямо скажем, не решает поставленную задачу, а лишь немного упрощает работу с ПДО. А так-то да, лучше сразу использовать второе

Answer 3

[index0h]

рекомендую посмотреть в сторону доктрины, да, по началу она кажется монструозной, но на практике все проще.

конкретно по вашей теме:

1. $where = '1 ; DROP DATABASE database_name'; И счастье радость, улыбки после анального секса))
2. что будет, если я передам не строки входными аргументами?
3. динамический $where... похоже вы делаете свою orm, я прав? Если да - не делайте, это уже есть и работает даже быстро и качественно))
4. НЕ ИСПОЛЬЗУЙТЕ ТРАНСЛИТ!!! Не опускайтесь до уровня 1С
5. if(, $table_name, function table_name(), $STH... Используйте PSR-2
6. Конструкцию
for($i = 0; $i < count($pole); $i++){
$que .= $pole[$i]." = ?, ";

}
$que = substr($que, 0, -2);

можно заменить на

$que = implode(' = ?,', $pole) . '= ?';

7. Если вам явно не нужен $i - используйте foreach
8. Поля все же стоит экранировать
9. Не нагружайте метод не нужной работой: если вам нужно работать со строкой - работайте, но в вашем случае нужен явно массив, так и принимайте только массив.
10. Вместо таких конструкций лучше либо используйте регулярки, либо проход с trim
$pole = explode(", ", $pole);
Если передать туда
$pole = 'a1, a2,\r a3';
Вы получите не совсем то, что ожидается.
preg_match_all('/[^\s\,]+/u', $pole, $matches);
11. Не используйте именование переменных в единичном представлении, когда работаете с множествами.
НЕ $pole, а $fieldList/$fields (в случае массива)
12. PDO != mysqli, для начала все же почитайте, какое API предлагает PDO, а потом его используйте. Просто то, что у вас сейчас - это как вилкой хлебать борщ.
13. И напоследок лучше - используйте ===, вместо ==