0

I want to make sure I don't have some memory-leak and bad coding-habits. I might just be answering myself here.

This is a typical function of mine, Should I Bind a $temp in all the return cases and then return it at the end instead or is this fine?

define DB_SERVER,DB_USERNAME,DB_PASSWORD,DB_DATABASE 
$db = mysqli_connect(DB_SERVER,DB_USERNAME,DB_PASSWORD,DB_DATABASE);

function Resolvestuff($input) {
    global $db;
    if ($stmt = $db->prepare("Select Column from Table where `a` = ?")) {
        $stmt->bind_param('s', $input);
        $stmt->execute();
        $stmt->store_result();
        $stmt->bind_result($col1);
        $row = $stmt->fetch();
        if ($stmt->num_rows == 0) {
            return "Nothing";
        } else {
            return $col1;
        }
        $stmt->free_result(); // i mean, is this even performed at some point ?
        $stmt->close();
    } else {
        return "Nothing";
    }

    //Return here instead ?
}
Dharman
  • 30,962
  • 25
  • 85
  • 135
  • You need to stop manually checking for errors. Please read: [Should we ever check for mysqli_connect() errors manually?](https://stackoverflow.com/q/58808332/1839439) and [Should I manually check for errors when calling “mysqli_stmt_prepare”?](https://stackoverflow.com/q/62216426/1839439) – Dharman Jun 23 '20 at 11:01
  • `$stmt->num_rows == 0` is not needed at all there – Dharman Jun 23 '20 at 11:01

1 Answers1

0

A simple answer to your questions is that $stmt is a local variable so PHP will clean it all up and garbage collect soon after the execution of the function is over. You don't need to call free_result or close.

The longer answer is that you approaching this from the wrong angle. You should not create specific functions like this. You must have one generic function. For example.

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$mysqli = new mysqli('localhost', 'user', 'pass', 'db');
$mysqli->set_charset('utf8mb4'); // always set the charset

function fetchSingle(mysqli $db, string $sql, array $params = []) {
    $stmt = $db->prepare($sql);
    if ($params) {
        $stmt->bind_param(str_repeat("s", count($params)), ...$params);
    }
    $stmt->execute();
    $stmt->bind_result($col1);
    $stmt->fetch();
    return $col1;
}

$val = fetchSingle($mysqli, "Select Column from Table where `a` = ?", [$input]);
Dharman
  • 30,962
  • 25
  • 85
  • 135
  • Wow, never thought of it like that, i can really see the potential for such wide-use function! But then again, i will come across Select Statements that is bound to be repeating in my site, and therefore a simplification in the shape of a premade Function for it would make sense no ? – deployHuman Jun 23 '20 at 11:42
  • No. SQL in itself is one line of code. Wrapping one line in a function is not very useful unless you have some other functionality to encapsulate too. Key thing here is that mysqli functions should not be used throughout your code. They must be encapsulated in an abstraction layer. mysqli is difficult to use and when you can you should use PDO instead. If you want to use mysqli you need to write functions like this which help you with simple things. Here is another good example https://phpdelusions.net/mysqli/simple – Dharman Jun 23 '20 at 11:45