1
\$\begingroup\$

I want to know if it is okay/safe to use a prepared statement and mysqli_num_rows like this:

public function getUnreadNumber() {
    $userLoggedIn = $this->user_obj->getUsername();

    // get_result for * and bind for select columns
    // bind_result Doesn't work with SQL query that use *
    $query = $this->con->prepare('SELECT * FROM notifications WHERE viewed="0" AND user_to = ? ');
    $query->bind_param("s", $userLoggedIn);
    $query->execute();
    $query_result = $query->get_result();

    return mysqli_num_rows($query_result);
}

Or should I do this?

$query = $this->con->prepare('SELECT * FROM notifications WHERE viewed="0" AND user_to = ? ');
$query->bind_param("s", $userLoggedIn);
$query->execute();
$query_result = $query->get_result();
$numRows = $query_result->num_rows;

return $numRows;
\$\endgroup\$
9
  • \$\begingroup\$ So is it ok if I use this with a prepared statement ? return mysqli_num_rows($query_result) \$\endgroup\$ Jul 15 '20 at 19:11
  • \$\begingroup\$ nusphere.com/kb/phpmanual/function.mysqli-num-rows.htm \$\endgroup\$
    – psaxton
    Jul 15 '20 at 19:26
  • 1
    \$\begingroup\$ That hasn't been updated since 06 \$\endgroup\$ Jul 15 '20 at 19:40
  • 2
    \$\begingroup\$ What are the possible values returned by this->user_obj->getUsername()?? \$\endgroup\$ Jul 16 '20 at 5:34
  • 2
    \$\begingroup\$ (If you are not ready to value feedback and criticism for any aspect of the code posted, your question is off-topic.) \$\endgroup\$
    – greybeard
    Jul 16 '20 at 11:11
6
\$\begingroup\$

You should definitely not be mixing procedural and object-oriented syntax.

Although it works with un-mixed syntax, the process is working harder than it needs to and delivering more result set data than you intend to use.

I would add COUNT(1) or COUNT(*) to the sql like this:

$sql = "SELECT COUNT(1) FROM notifications WHERE viewed='0' AND user_to = ?";
$query = $this->con->prepare($sql);
$query->bind_param("s", $userLoggedIn);
$query->execute();
$query->bind_result($count);
$query->fetch();
return $count;

Assuming the sql is not broken due to syntax errors, this will always return a number.

\$\endgroup\$
0
3
\$\begingroup\$

@mickmackusa is correct, you should never ever use num_rows to count the rows in the database, it could kill your database server. This function is rather useless for any other use case too.

Besides, always follow the rule of thumb: make a database to do the job. If you need tell a database to give you the count.

As a code improvement, let me suggest you a mysqli helper function I wrote to put all the prepare/bind/execute business under the hood

public function getUnreadNumber()
{
    $userLoggedIn = $this->user_obj->getUsername();
    $sql = "SELECT COUNT(1) FROM notifications WHERE viewed='0' AND user_to = ?";
    return prepared_query($this->con, $sql, $userLoggedIn)->get_result()->fetch_row()[0];
}
\$\endgroup\$
1
  • \$\begingroup\$ If you need a count tell a database to give you the count.? \$\endgroup\$
    – greybeard
    Jul 17 '20 at 17:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.