Connection success but can't assign it

[TechnoDiver]

Could someone explain to me why this connection/query succeeds

<?php
DB::getInstance()->query("SELECT username FROM users WHERE username=?", array("TechnoDiver"));

But when I do this it comes back as failed

<?php
$user = DB::getInstance()->query("SELECT username FROM users WHERE username=?", array("TechnoDiver"));

if($user) {
    echo "success -> ";
} else {
    echo "fail -> ";
}

like I said I ran tests for the DB->query in the query method and it comes back successfully. It's only when I try to assign it that it comes back failed. Why??

[kicken]

What is your code for DB::getInstance() and your query() methods?

[TechnoDiver]

1 hour ago, kicken said:

What is your code for DB::getInstance() and your query() methods?

DB::getInstance() is ->

<?php
public static function getInstance() {
        if(!isset(self::$_instance)) {
            self::$_instance = new DB();
        }
        return self::$_instance;
    }

query() is ->

<?php
public function query($sql, $params = array()) {
        $this->_error = false;
        if($this->_query = $this->_pdo->prepare($sql)) {
            $x = 1;
            if(count($params)) {
                foreach($params as $param) {
                    $this->_query->bindValue($x, $param);
                    $x++;
                }
            }

            if($this->_query->execute()) {
                $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ);
                $this->_count = $this->_query->rowCount();
                echo "query success <br>";
            } else {
                $this->_error = true;
                echo "query fail";
            }
        }
    }

when I run

<?php
$user = DB::getInstance()

this is successful ->

<?php
if($user) {
    echo "success -> ";
    // echo $user;
} else {
    echo "fail -> ";
    // echo $user;
}

when I make it ->

$user = DB::getInstance()->query("SELECT username FROM users WHERE username=?", array("TechnoDiver"));

than this fails ->\

if($user) {
    echo "success -> ";
    // echo $user;
} else {
    echo "fail -> ";
    // echo $user;
}

But the echo in query() always comes back "query success"

can't figure out why adding

->query("SELECT username FROM users WHERE username=?", array("TechnoDiver"))

collapses the methods

Edited September 4, 2021 by TechnoDiver

[kicken]

Your query method does not return anything, so $user will be NULL.

[Solar]

Why are you re-initializing the following $params array?

public function query($sql, $params = array()) {

Should be just this, no?

public function query($sql, $params) {

 

Edited September 4, 2021 by Solar

[TechnoDiver]

31 minutes ago, Solar said:

Should be just this, no?

public function query($sql, $params) {

doing it like that means that $params is required. The way I did it means that $params can be there or not

[maxxd]

Pardon the unsolicited advice, but unless your DB class does something really special it looks like you're making things far more difficult than they need to be. PDO is already an abstraction class; just use it. Your query() method and call can be done as such:

$qry = "
	SELECT	 username
	FROM users
	WHERE username = ?
";

$sql = $pdo->prepare($qry);
$sql->execute(['TechnoDiver']);

die("<pre>".var_export($sql->fetchAll(), true)."</pre>");

And obviously your query is meant for SELECT statements, but keep in mind if you're planning on extending to INSERT or UPDATE the performance benefits of prepare(). From the documentation:

Quote

Calling PDO::prepare() and PDOStatement::execute() for statements that will be issued multiple times with different parameter values optimizes the performance of your application by allowing the driver to negotiate client and/or server side caching of the query plan and meta information.

So, at that point you're forced to recreate the prepare() method in order to bind multiple arrays of values to the same query.

[Strider64]

From what I am see is that you are doing too much inside methods and even the classes. Keep methods simple as possible such as the following example:

    public static function page($perPage, $offset, $loc = 'index'): array
    {
        $sql = 'SELECT * FROM ' . static::$table . ' WHERE page=:page ORDER BY date_updated DESC LIMIT :perPage OFFSET :blogOffset';
        $stmt = Database::pdo()->prepare($sql); // Prepare the query:
        $stmt->execute(['perPage' => $perPage, 'blogOffset' => $offset, 'page' => $loc]); // Execute the query with the supplied data:
        return $stmt->fetchAll(PDO::FETCH_ASSOC);
    }

Classes can be inherited from a parent class or even another child class (However, I wouldn't go too deep) There are other ways of doing inheritance as well and some people frown on doing the way I do, but from what I do is small compared to larger development places.

class CMS extends DatabaseObject
{
    protected static string $table = "cms"; // Table Name:
    static protected array $db_columns = ['id', 'user_id', 'thumb_path', 'image_path', 'Model', 'ExposureTime', 'Aperture', 'ISO', 'FocalLength', 'author', 'heading', 'content', 'data_updated', 'date_added'];
    public $id;
    public $user_id;
    public $page;
    public $thumb_path;
    public $image_path;
    public $Model;
    public $ExposureTime;
    public $Aperture;
    public $ISO;
    public $FocalLength;
    public $author;
    public $heading;
    public $content;
    public $date_updated;
    public $date_added;

    /*
     * Create a short description of content and place a link button that I call 'more' at the end of the
     * shorten content.
     */
    #[Pure] public static function intro($content = "", $count = 100): string
    {
        return substr($content, 0, $count) . "...";
    }

I do that with my PDO Database Connection that I call the Grandparent (called Database and I utilize it by doing Database::pdo() ).  I go by the the K.I.S.S. (Keep It Simple Stupid) model as I find out that I write less code and can be used over and over again. I find myself (as I sure others do to) when I try to get fancy and write a bunch of code it leads to write more code with the class end up being longer than they should. Granted having a long class is sometimes unavoidable, but even them after you gain more knowledge of PHP can be streamline. I learn something new everyday in PHP coding (and other languages) and sometimes I slap myself why didn't I think of that. 🤣

[TechnoDiver]

17 hours ago, kicken said:

Your query method does not return anything, so $user will be NULL.

I had already tried this.

I tried return $this->_errors. It should have been just return $this; So now it works

But it's interesting because the below change works with query() as it was - not returning anything

<?php
$user = DB::getInstance();//->query("SELECT username FROM users WHERE username=?", array("TechnoDiver"));
$user->query("SELECT username FROM users WHERE username=?", array("TechnoDiver"));

Which leads into another question of why? I have no idea why it works breaking the line up like this while query() doesn't have a return. Maybe someone here can shine some light on that.

To the respondents suggesting something more simple, thanks for your responses. Solicited or not all advice is good, everyone has something to learn from.

It is a bit more complex, yes, but it's a class not meant for a single project, it's me trying my hand at a very abstract database wrapper. There's not meant to be any sql statements in this class.

Thanks for the responses phreaks. The answer was indeed to either 'return $this' in query() or for some obscure reason, that I'm hoping someone here will be able to answer, to leave query() alone and split that statement into 2 lines.

 

[kicken]

2 hours ago, TechnoDiver said:

But it's interesting because the below change works with query() as it was - not returning anything

You're not trying to use a return value from your query method in that code.  Notice the line containing the call to query doesn't contain a $something = in front of it.

You're DB::getInstance() method does return a value, which gets assigned to $user in that code.  In the single-line version of the code, the value returned by DB::getInstance() is not captured, just used temporarily for the call to query.   If you wanted a multi-line equivalent of your single line version it would be:

$tmp = DB::GetInstance();
$user = $tmp->query('...');

if($user) {
    echo "success -> ";
} else {
    echo "fail -> ";
}