PHP MySQL class

[Mac II]

Hello today i will show you a simple PHP MySQL class and post your comment for this class .

PHP Code:

<?php
class ConnectMySQL {
    Var $dbhost;
    Var $dbuser;
    Var $dbpass;
    Var $dbname;
    Var $connect;
    Var $selectdb;
    function __Construct($dbhost, $dbuser, $dbpass, $dbname) {
        if(isset($dbhost) and isset($dbuser) and isset($dbpass) and isset($dbname)):
            $this->connect = mysql_connect($dbhost, $dbuser, $dbpass);
            $this->selectdb = mysql_select_db($dbname);
            return $this->connect;
            return $this->selectdb;
        else:
            Exit;
        Endif;
    }
}
$connect = new ConnectMySQL("localhost", "root", "pass", "db");
?>

Thanks!

[Inigoesdr]

• You shouldn't use "var" to declare your variables because it's ambiguous and the functionality could change -- use public/private/protected/static
• Most of your variables don't need to be declared at the class level if you are only using them in the one function
• Your constructor should be named "__construct" -- lowercase. It works uppercase, but the proper notation is lower. Same goes for your exit/endif keywords. You write VB, don't you?
• Why do you have a class with a single method? You should add methods to do more(query, num rows, etc.) or just make it a function(assuming it's something that needs to be repeated).

[Mac II]

1. I tried with private / public but returning this error : Parse error: syntax error, unexpected T_STRING, expecting T_OLD_FUNCTION or T_FUNCTION or T_VAR or '}' in /homepages/26/d94605010/htdocs/lz/writecodeonline.com/php/index.php(98) : eval()'d code on line 2

I dont know why...

2 and 4. Yeh , this is just a simple mysql class , i am going to make advanced one soon
3.I like coding uppercase but sometimes i dont.

[Fou-Lu]

Scope would replace the var completely. If your constructor is working, you are on a PHP5.x environment, so members should be declared as private $dbhost; for example. Don't use public in PHP for non-final members ever. Since its datatype weak its more work to hack together a __set than it is to provide accessors and mutators for each member.

Constructors do not return results, they return a reference to the created object (which I believe is actually copied from the new to the variable, and void is actually returned from the __construct). You cannot return $this->connect or $this->selectdb, and in a standard method/function you can only return one.

I wouldn't have any of these as members at all. That exposes a privacy problem, since var_dump and print_r can both see private members, this poses a security risk on systems that allow external developers to build on. Just passing the parameters is fine, all you need to track is a $connection variable.

Edit:
Also, I should mention that you are better off not exiting / dying from a method at all. You would be better throwing an exception and letting the calling scope determine what do to with it.

[Mac II]

New code :

PHP Code:

class MySQL {
    public $array = array();
    function __construct($array[1], $array[2], $array[3], $array[4]) {
        if(isset($array[1]) and isset($array[2]) and isset($aray[3]) and isset($array[4])):
            mysql_connect($array[1], $array[2], $array[3]);
            mysql_select_db($array[4]);
            if(mysql_error()):
                echo mysql_error();
            endif;
        else:
            throw new exception('Required fields not set.');
        endif;
    }function makeQuery($string) {
        return mysql_query($string);
    }
    function num_rows($string) {
        return mysql_num_rows($string);
    }
    function fetch_array($query) {
        return mysql_fetch_array($query);
    }
}
$mysql = new MySQL("host", "root", "pass", "db");
$ff = $mysql->makeQuery("SELECT * FROM table");
echo $mysql->num_rows($ff);
$row = $mysql->fetch_array($ff);
echo $row['first']; 


I haven't tested it , if you find errors post it.

[Fou-Lu]

No, this won't work either. You cannot specify array access parameters in a function or method, that just doesn't make any sense.
Isset checks are pointless. You have not provide a means to escape from providing data to a function (empty is technically fine, the only thing you need to establish a database connection is the host). I'd use AND very sparingly as well. AND has a lower priority than || (but not OR) which both AND and OR have a lower priority than assignment. Use && and || unless you are intending to use the lower priority of the AND or OR.
This entire class is easily written as simply:

PHP Code:

<?php

class MySQLConnection // Don't leave it under just MySQL
{
    private $rConn;
    public function __construct($host, $user = '', $pass = '', $dbname = '')
    {
        @mysql_connect($host, $user, $pass) or throw new Exception(mysql_error());
        if (!empty($dbname))
        {
            $this->selectdb($dbname);
        }
    }

    public function selectdb($dbname)
    {
        @mysql_select_db($dbname, $this->rConn) or throw new Exception(mysql_error());
    }

    // Whatever else.
}

MySQL itself is getting old. You are better off moving to either a PDO or MySQLi environment instead, both of which are object oriented by default.

Edit:
Lame, you can't throw from an or.
This will work as intended.

PHP Code:

<?php

class MySQLConnection // Don't leave it under just MySQL
{
    private $rConn;
    public function __construct($host, $user = '', $pass = '', $dbname = '')
    {
        if (!@mysql_connect($host, $user, $pass))
        {
            throw new Exception(mysql_error());
        }
        if (!empty($dbname))
        {
            $this->selectdb($dbname);
        }
    }

    public function selectdb($dbname)
    {
        if (!@mysql_select_db($dbname, $this->rConn))
        {
            throw new Exception(mysql_error());
        }
    }

    // Whatever else.
}
?>