Am I inscure?

[L4DD13]

Hi guys,

Ok... erm

Right...

Im building a base class for my website that is going to contain all the basic information for my site, like the header, footer, mysql connection (which is going to be in a separate class).

The mysql connection is what I'm unsure about, If I set the connection variables (ie. mysql username, password, host, etc) to private and made the actual connector ($connect = mysql_connect(xxxxx)) public, would that be secure of not? It's just that its getting a bit tedious having to make mysql connections for each class (i mean having to call my mysql class).

To put it short, I want to make a connection to my mysql server in the class "base" and for all the classes that extend the "base" class to be be able to access the connection without having to make a new one, would setting the variables to private and the connector to public be secure or not?

Thanks.

[AnthonySterling]

Have your base class request an 'instance' of your database connection, you should not really have to connect in every child class.

[AnthonySterling]

Something similar to:-

<?php

class Database
{
    private static $oInstance = null;
    
    public static function getInstance()
    {
        if(is_null(self::$oInstance))
        {
            self::$oInstance = new Database( 'localhost', 'username', 'password', 'database' );
        }
        return self::$oInstance;
    }
    
    private function __construct( $sHost, $sUsername, $sPassword, $sDatabase, $iPort = 3306 )
    {
        try{
            self::$oInstance = new mysqli( $sHost, $sUsername, $sPassword, $sDatabase, $iPort );
            return;
        }catch(Exception $oException){
            throw new Exception($oException->getMessage());
            return;
        }
    }
}

class CMS
{
    protected $oDatabase;
    
    public function __construct()
    {
        $this->oDatabase = Database::getInstance();
    }
}

class Users extends CMS
{
    public function getUserByID( $iID )
    {
        $oResult = $this->oDatabase->query("SELECT name,email FROM users WHERE id = $iID LIMIT 1;");
        // Do Stuff.
    }
}

?>

I hope I'm making sense, or should I say, talking sense!

[L4DD13]

Thanks, That makes sense.

[AnthonySterling]

Great, just give me a shout if you stuck.

[L4DD13]

How do i pass to the database class?

[AnthonySterling]

How do you mean?

You don't have to pass the database class to any other anymore, the getInstance() method will automatically create/return it for you.

[L4DD13]

Actually it doesn't matter, i've come realised that I don't need to pass to the database class.

[L4DD13]

Is mysqli different to mysql?

[decowski]

Is mysqli different to mysql?

It is. It's still a wrapper to MySQL, however, so you can use either one. The i stands for improved. It also comes in OO flavour. Recommended over the mysql_* functions.

[L4DD13]

im lame i know, but I cant seem to get the script to do querys

[L4DD13]

I get errors,

When i use: $settings_query = mysqli_query( $this->_oDatabase , "SELECT * FROM `"._settingstbl."");
I get: mysqli_query() expects parameter 1 to be mysqli, null given in C:\xampp\htdocs\classes\class.base.php on line 22

And,
When I use: $get_settings = $this->_oDatabase->query("SELECT * FROM `"._settingstbl."`");
I get: Fatal error: Call to a member function query() on a non-object in C:\xampp\htdocs\classes\class.base.php on line 21

[decowski]

You should use the latter.

When I use: $get_settings = $this->_oDatabase->query("SELECT * FROM `"._settingstbl."`");
I get: Fatal error: Call to a member function query() on a non-object in C:\xampp\htdocs\classes\class.base.php on line 21

Now this means that the $_oDatabase property is not a properly instantiated object. In his example, SilverBulletUK used $oDatabase (without the underscore). Make sure you're using the same property names.

[L4DD13]

This is the script:

<?php

class database
{

    private static $oInstance = null;
    public $_config;

    public static function getInstance()
    {
        if(is_null(self::$oInstance))
        {
            require_once("../cwp-config.php");
            self::$oInstance = new Database( DBHOST, DBUSERNAME, DBPASSWORD, DEFAULT_DATABASE );
        }
        return self::$oInstance;
    }

    private function __construct($sHost, $sUsername, $sPassword, $sDatabase, $iPort = 3306)
    {
        try{
            self::$oInstance = new mysqli( $sHost, $sUsername, $sPassword, $sDatabase, $iPort );
            return;
        }catch(Exception $oException){
            throw new Exception($oException->getMessage());
            return;
        }

    }

}

class base
{

    protected     $oDatabase;// Database connection
    public         $_filebase;    // The absolute path to the website on the server.
    public        $_urlbase;     // The base url of the website.
    public        $_config;     // Location of the config.

    public function __construct()
    {
        
        $this->oDatabase = Database::getInstance();

        $this->_filebase            = $_SERVER['DOCUMENT_ROOT'] . "/";
        $this->_urlbase                = "http://" . $_SERVER['HTTP_HOST'];
        $this->_config                = "../cwp-config.php";

        require_once($this->_config);

        // Database Connection
        $get_settings = $this->oDatabase->query("SELECT * FROM `"._settingstbl."`");
        if(mysqli_num_rows($get_settings))
        {
            echo 'settings';
        } else {
            echo 'no settings';
        }

    }

    public function _header()
    {}

    public function _footer()
    {}

    public function _urlbase(){ return $this->_urlbase; }

}

?>

[decowski]

In Database constructor check if the object has been created before returning it. If it hasn't been created then check for errors. You're probably passing wrong database details to the function (host, username, password).

Also at the top of your script add:

error_reporting(E_ALL | E_STRICT); 

[L4DD13]

Try this modified, Database class...

class Database
{
    private static $oInstance = null;
    
    public static function getInstance()
    {
        if(is_null(self::$oInstance))
        {
            self::$oInstance = new Database( 'localhost', 'username', 'password', 'database' );
        }
        return self::$oInstance;
    }
    
    private function __construct( $sHost, $sUsername, $sPassword, $sDatabase, $iPort = 3306 )
    {
        try{
            self::$oInstance = new mysqli( $sHost, $sUsername, $sPassword, $sDatabase, $iPort );
            if( ! is_object(self::$oInstance) || self::$oInstance->connect_error )
            {
                throw new Exception(mysqli_connect_error());
            }
            return;
        }catch(Exception $oException){
            throw new Exception($oException->getMessage());
            return;
        }
    }
} 

I'm not entirely sure if the mysqli object throws an exception when it doesn't connect, so I just added a little routine.

In Database constructor check if the object has been created before returning it. If it hasn't been created then check for errors. You're probably passing wrong database details to the function (host, username, password).

Also at the top of your script add:

error_reporting(E_ALL | E_STRICT); 

After implementing both of your suggestions i get the following error:
Fatal error: Call to undefined method Database::query() in C:\xampp\htdocs\classes\class.base.php on line 86

[AnthonySterling]

Try this modified, Database class...

class Database
{
    private static $oInstance = null;
    
    public static function getInstance()
    {
        if(is_null(self::$oInstance))
        {
            self::$oInstance = new Database( 'localhost', 'username', 'password', 'database' );
        }
        return self::$oInstance;
    }
    
    private function __construct( $sHost, $sUsername, $sPassword, $sDatabase, $iPort = 3306 )
    {
        try{
            self::$oInstance = new mysqli( $sHost, $sUsername, $sPassword, $sDatabase, $iPort );
            if( ! is_object(self::$oInstance) || self::$oInstance->connect_error )
            {
                throw new Exception(mysqli_connect_error());
            }
            return;
        }catch(Exception $oException){
            throw new Exception($oException->getMessage());
            return;
        }
    }
} 

I'm not entirely sure if the mysqli object throws an exception when it doesn't connect, so I just added a little routine.

[AnthonySterling]

Doh!

My bad, sorry! That's what I get for quickly knocking up code, then just assuming it's correct...apologies.

Thanks Decowski.

[L4DD13]

One last thing...

Using error_reporting(E_ALL | E_STRICT); is stopping me from using the likes of mktime().
Is there anyway around it?

[decowski]

One last thing...

Using error_reporting(E_ALL | E_STRICT); is stopping me from using the likes of mktime().
Is there anyway around it?

How is it stopping you?

[L4DD13]

@SilverBulletUK

Can you tell me why modification of your class isn't working?

__DBSETTINGS__ is the script containing the connection variables.

<?php

class Database
{

    private static $oInstance = null;

    public static function getInstance()
    {
        require_once(__DBSETTINGS__);
        if(is_null(self::$oInstance)) {
            new Database( DBHOST, DBUSERNAME, DBPASSWORD, DEFAULT_DATABASE );
        }

        return self::$oInstance;

    }

    private function __construct( $sHost, $sUsername, $sPassword, $sDatabase, $iPort = 3306 )
    {


        try{

            $connect = @mysqli_connect( $sHost, $sUsername, $sPassword, $sDatabase, $iPort);
            if(is_resource($connect)){
                self::$oInstance = new mysqli( $sHost, $sUsername, $sPassword, $sDatabase, $iPort );
            }
            return;
        } catch (Exception $oException){
            throw new Exception($oException->getMessage());
            return;
        }

    }

}

?>

Thanks

[L4DD13]

Im getting an error/warning that i've never seen before, im getting the following when i use date("Y", mktime()):


Strict Standards: mktime() [function.mktime]: It is not safe to rely on the system's timezone settings. Please use the date.timezone setting, the TZ environment variable or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'Europe/London' for '0.0/no DST' instead in C:\xampp\htdocs\classes\class.base.php on line 87

Strict Standards: mktime() [function.mktime]: You should be using the time() function instead in C:\xampp\htdocs\classes\class.base.php on line 87

Strict Standards: date() [function.date]: It is not safe to rely on the system's timezone settings. Please use the date.timezone setting, the TZ environment variable or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'Europe/London' for '0.0/no DST' instead in C:\xampp\htdocs\classes\class.base.php on line 87

[decowski]

Im getting an error/warning that i've never seen before, im getting the following when i use date("Y", mktime()):


Strict Standards: mktime() [function.mktime]: It is not safe to rely on the system's timezone settings. Please use the date.timezone setting, the TZ environment variable or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'Europe/London' for '0.0/no DST' instead in C:\xampp\htdocs\classes\class.base.php on line 87

Strict Standards: mktime() [function.mktime]: You should be using the time() function instead in C:\xampp\htdocs\classes\class.base.php on line 87

Strict Standards: date() [function.date]: It is not safe to rely on the system's timezone settings. Please use the date.timezone setting, the TZ environment variable or the date_default_timezone_set() function. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'Europe/London' for '0.0/no DST' instead in C:\xampp\htdocs\classes\class.base.php on line 87

Well, it pretty much explains it. You need to set the timezone using one of the methods specified.

You should also use time() function instead of mktime() if you want the current timestamp. mktime() is only to be used if you want a timestamp for a specified time/date.

[L4DD13]

I dont know why i do that, i get an error and pass the problem onto someone else. I need to start reading them 1st. Thanks for the pointer though.

[decowski]

I dont know why i do that, i get an error and pass the problem onto someone else. I need to start reading them 1st. Thanks for the pointer though.

Realising the problem is the first step to solving it

BTW: They aren't errors as such. They are just standard notices. The code will work if you remove E_STRICT reporting but it's best practice to code with the set rules (some of your code may not work in future versions of PHP if you don't).