PHP MySQL class

[Mac II]

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

<?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.


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 :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

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.


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

<?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.
}
?>