Skip to main content
added 84 characters in body
Source Link
Your Common Sense
  • 8.8k
  • 1
  • 21
  • 51

There are two main points of improvement:

  1. First of all, never ever use die(error message) in your scripts. Neither try/catch should be ever used only to echo the error message out. You'd be surprised, but you will have more informative error message if just get rid of the whole try/catch/die stuff. Please read my article on PHP error reporting for the details

  2. PDO code itself could be made more tidy, thanks to various helper functions PDO offers

So the whole code should be

$sql_ip = inet_pton($ip); // IPV6

$stmt = $pdo->prepare('SELECT 1 FROM sessions WHERE s_ipv4 = ? OR s_ipv6 = ?');
$stmt->execute([$sql_ip,$sql_ip]);

if ($stmt->fetchColumn()) {
    // Do something
} else {
    // No rows
}

Here I used positional placeholders for brevity, and sent your variables directly to execute for the same purpose. 

Also I recommend to avoid rowCount()rowCount() function as it makes very little sensewould be more logical: in your code you are selecting some data, but never use it. Instead, I would suggest to use the actual data selected (just a literal "1" in your case, fetched directly using fetchColumn()fetchColumn() method).

There are two main points of improvement:

  1. First of all, never ever use die(error message) in your scripts. Neither try/catch should be ever used only to echo the error message out. You'd be surprised, but you will have more informative error message if just get rid of the whole try/catch/die stuff. Please read my article on PHP error reporting for the details

  2. PDO code itself could be made more tidy, thanks to various helper functions PDO offers

So the whole code should be

$sql_ip = inet_pton($ip); // IPV6

$stmt = $pdo->prepare('SELECT 1 FROM sessions WHERE s_ipv4 = ? OR s_ipv6 = ?');
$stmt->execute([$sql_ip,$sql_ip]);

if ($stmt->fetchColumn()) {
    // Do something
} else {
    // No rows
}

Here I used positional placeholders for brevity, and sent your variables directly to execute for the same purpose. Also I recommend to avoid rowCount() function as it makes very little sense. Instead, use the actual data selected (just a literal "1" in your case, fetched directly using fetchColumn() method)

There are two main points of improvement:

  1. First of all, never ever use die(error message) in your scripts. Neither try/catch should be ever used only to echo the error message out. You'd be surprised, but you will have more informative error message if just get rid of the whole try/catch/die stuff. Please read my article on PHP error reporting for the details

  2. PDO code itself could be made more tidy, thanks to various helper functions PDO offers

So the whole code should be

$sql_ip = inet_pton($ip); // IPV6

$stmt = $pdo->prepare('SELECT 1 FROM sessions WHERE s_ipv4 = ? OR s_ipv6 = ?');
$stmt->execute([$sql_ip,$sql_ip]);

if ($stmt->fetchColumn()) {
    // Do something
} else {
    // No rows
}

Here I used positional placeholders for brevity, and sent your variables directly to execute for the same purpose. 

Also I recommend to avoid rowCount() function as it would be more logical: in your code you are selecting some data, but never use it. Instead, I would suggest to use the actual data selected (just a literal "1" in your case, fetched directly using fetchColumn() method).

Source Link
Your Common Sense
  • 8.8k
  • 1
  • 21
  • 51

There are two main points of improvement:

  1. First of all, never ever use die(error message) in your scripts. Neither try/catch should be ever used only to echo the error message out. You'd be surprised, but you will have more informative error message if just get rid of the whole try/catch/die stuff. Please read my article on PHP error reporting for the details

  2. PDO code itself could be made more tidy, thanks to various helper functions PDO offers

So the whole code should be

$sql_ip = inet_pton($ip); // IPV6

$stmt = $pdo->prepare('SELECT 1 FROM sessions WHERE s_ipv4 = ? OR s_ipv6 = ?');
$stmt->execute([$sql_ip,$sql_ip]);

if ($stmt->fetchColumn()) {
    // Do something
} else {
    // No rows
}

Here I used positional placeholders for brevity, and sent your variables directly to execute for the same purpose. Also I recommend to avoid rowCount() function as it makes very little sense. Instead, use the actual data selected (just a literal "1" in your case, fetched directly using fetchColumn() method)