CODEWITHSUNDEEP
phpoop
user1311784
user1311784

Reputation: 345

variables in classes - correct approach

This code will output the same. My question is, what is the correct way to do this. The first approach or the second? Or there is any better way? I don't see any advantage of one Class over the other. 

<?php
    class Client{
        var $id;
        var $email;

        function __construct($id){
            $this->id=$id;
        }

        public function get_email($db){
            $sql = $db -> prepare(" SELECT email FROM users  WHERE id = ? ");

            $sql -> bind_param('i', $this->id);
            $sql->execute();

            $sql->bind_result($email);

            if ($sql -> fetch()) {
                return $this->email=$email;
            }
            else
            return false;
        }
    }

    class Client_{
        public function get_email($db, $id){
            $sql = $db -> prepare(" SELECT email FROM users  WHERE id = ?");

            $sql -> bind_param('i', $id);
            $sql->execute();

            $sql->bind_result($email);

            if ($sql -> fetch()) {
                return $email;
            }
            else
            return false;
        }
    }
    ?>

index.php

<?php
$Client = new Client(1);
$a = $Client -> get_email($db);

print_r($a);

$Client_ = new Client_();
$b = $Client_ -> get_email($db, 1);

print_r($b);
?>

Upvotes: 0

Views: 109

Answers (3)

tereško
tereško

Reputation: 58444

Please stop using var to define class variables in php. It is not 4.x anymore. Now where have public, private and protected.

That said, the instance of Client require both connection and identificator:

class Client
{
    protected $connection;
    protected $id;
    protected $email = null;

    public function __construct($connection, $id)
    {
        $this->connection = $connection;
        $this->id = $id
    }

    public function getEmail()
    {
        if ( $this->email === null )
        {
            $query = 'SELECT email FROM users WHERE id = ?';
            $statement = $this->connection->prepare( $query );
            $statement->bind_param('i', $this->id);
            if ( $statement->execute() )
            {
                 $statement->bind_result($this->email);
            }
        }
        return $this->email;
    }

}

P.S. i would actually prefer PDO instead of MySQLi for connection API. If for not other reason, then just because it has more flexibility in error handling.

Upvotes: 0

Sampo Sarrala
Sampo Sarrala

Reputation: 4868

In second approach it does not make sense to instantiate class, as there is nothing to be stored for future use. So it would be better to use "static class" if varying data is stored somewhere else: 

class Client_{
    static public function get_email($db, $id){
        $sql = $db -> prepare(" SELECT email FROM users  WHERE id = ?");

        $sql -> bind_param('i', $id);
        $sql->execute();

        $sql->bind_result($email);

        if ($sql -> fetch()) {
            return $email;
        }
        else
        return false;
    }
}

// And use it static way without instantiating first:
Client_::get_email( $arg1, $arg2 );

If i shoul make decision between those two I'm going to take first one.

I dont know how you are going to use either one of those classes but still for me it makes more sense to store $db and supply $id from outside and make $email local: 

class Client{
    var $db;

    function __construct($db){
        $this->db=$db;
    }

    public function get_email($id){
        $sql = $this->db -> prepare(" SELECT email FROM users  WHERE id = ? ");

        $sql -> bind_param('i', $id);
        $sql->execute();

        $sql->bind_result($email);

        if ($sql -> fetch()) {
            return $email;
        }
        else
        return false;
    }
}

Also changed this line: return $this->email=$email;, maybe I got it wrong but I think that it just doesn't make sense.

Upvotes: 1

Hubro
Hubro

Reputation: 59333

I'd say the first one is more "correct" in this scenario since the class name indicates that the class is a model for a database table, meaning that most or all changes to the table should be abstracted through the class. Often the instantiation of the model class will represent one row of the database table, so it makes sense that the id is a member of the class.

I would also pass the database connection to the model through the constructor, or somehow have it globally available.

^ Just my 2 cents

Upvotes: 0

Related Questions