Имеет ли смысл такая проверка?

Question

[Алексей Коновалов]

Всем привет! Подскажите плз, правильна ли такая конструкция и точнее нужна ли проверка на существование insert_id?

public function createUserFromSocialId($provider, $social_id, $name, $email){

		$provider 	= $this->db->real_escape_string($provider);
		$social_id 	= intval($social_id);
		$name 		= $this->db->real_escape_string($name);
		$email 		= $this->db->real_escape_string($email);

		if($this->db->query("INSERT INTO `users` (`name`, `email`) VALUE('{$name}','{$email}')")){
			$user_id = $this->db->insert_id;

			if(!empty($user_id)){
				$query = "INSERT INTO `users_social` (`provider`, `social_id`, `user_id`) 
						  VALUE('{$provider}', '{$social_id}','{$user_id}')";

				if($this->db->query($query)){
					return true;
				}

				$this->db->query("DELETE FROM `users` WHERE `id` = {$user_id}");
			}

			
		}

		return false;

	}

Comments

[Дмитрий]

Добрый день.
Ну почему бы и не проверять. Есть запись в таблицу users есть - продолжаем код.
Думаю, что Вам надо использовать транзакции, тем самым Вы можете избежать выполнение delete если запись в users_social не удалась.
https://habr.com/ru/post/446662/

p.s. В ответах не рекомендуют полагаться на значение id, поэтому поправлю себя, проверку надо делать, но на успешность выполнения запроса, выполнился он или нет. Транзакции облегчат задачу.

Answer 1

[Nikolay ResviL]

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

Comments

[Алексей Коновалов]

Спасибо за ответ! Попробовал переписать код под использование транзакций... Мне кажется он стал выглядеть куда более страшным чем с if

public function createUserFromSocialId($provider, $social_id, $name, $email){

		$provider 	= $this->db->real_escape_string($provider);
		$social_id 	= intval($social_id);
		$name 		= $this->db->real_escape_string($name);
		$email 		= $this->db->real_escape_string($email);

		try{
			$this->db->begin_transaction();

			if(!$this->db->query("INSERT INTO `users` (`name`, `email`) VALUE('{$name}','{$email}')")){
				throw new \Exception;
			}

			$user_id = $this->db->insert_id;

			if(empty($user_id)){
				throw new \Exception;
			}

			if(!$this->db->query("
				INSERT INTO `users_social` (`provider`, `social_id`, `user_id`) 
				VALUE('{$provider}', '{$social_id}','{$user_id}')")
			){
				throw new \Exception;
			}

			$this->db->commit();
		}catch(\Exception $e){
			$this->db->rollback();
		}

		return false;

	}

[Nikolay ResviL]

Алексей Коновалов, только сообщения к исключениям добавить нужно с описанием ошибки. а так куда приятнее стало, чем куча вложений if'ов друг в друга(:

[edward_freedom]

Николай Романов, после точки, предложение начинается с заглавной буквы

[Nikolay ResviL]

edward_freedom, ну это уже лишь бы к чему привязаться(:

[edward_freedom]

Николай Романов, так это не моя прихоть, правила языка такие, я то тут при чем

[Алексей Коновалов]

edward_freedom, Что тут делает гуманитарий?

[edward_freedom]

Алексей Коновалов, я не знаю, что Николай Романов тут забыл