csclanteam Posted November 16, 2008 Share Posted November 16, 2008 hello people i'm new to oop and i'm trying to build from scratch an oop members system i've made a part of the code but i run into some trouble when i tried to login it doesn't give me any error .. or message.. can someone please help me ? mysql class: <?php class MySQL { var $totalTime = 0; function MySQL() { global $config; $host = $config['db_host']; $db_user = $config['db_user']; $db_pass = $config['db_password']; $dba = $config ['db_database']; $startTime = $this->getMicroTime(); mysql_connect($host, $db_user, $db_pass) or die("Could Not Connect To DataBase"); mysql_select_db($dba) or die("Could Not Select DataBase"); $this->totalTime += $this->getMicroTime() - $startTime; } // MySQL Query Command function Query($sql) { $startTime = $this->getMicroTime(); ++$this->queryCounter; $result = mysql_query($sql); if (mysql_error()) { $this->mailError($sql, mysql_error(), mysql_errno()); return $this->error("<br />There was a SQL error and it has been reported. Sorry for the inconvience. <br />"); } $this->totalTime += $this->getMicroTime() - startTime; return $result; } // MySQL Fetch Arrray Query function FetchArray($result) { $rows = mysql_fetch_array($result); return $rows; } // mysql_num_rows function NumRows($result) { $count = mysql_num_rows($result); return $count; } } ?> the next parts of the php are connected togther. this part it's checking what form has been submited and then goes to onther function: <?php class Process { function Process() { global $session; if(isset($_POST['login'])) { $this->Login(); } function Login() { global $session; $cmd = $session->Login($_POST['user'], $_POST[pass], isset($_POST['remember'])); if($cmd) { header ("Location:index.php"); } } ?> the next part is where i start my session and include it in index.php <?php class Session { function Session() { $this->time = time(); $this->StartSession(); } function StartSession() { global $users; session_start(); $this->logged_in = $this->CheckLogin(); } function Login($Luser, $Lpass, $Lremember) { global $users, $form; $field = "user"; // Username Name Field. if(!$Luser || strlen($subuser = trim($Luser)) == 0) { $error['test']; } else { if(!eregi("^([0-9a-z])*$", $Luser)) { $error['test']; } } $field = "pass"; if(!$Lpass) { $error['test']; } if($form->num_errors > 0) { return false; } $Luser = stripslashes($Luser); $result = $users->ConfirmLogin($Luser, md5($Lpass)); if($result == 1) { $field = "user"; $error['test']; } else if($result == 2) { $field = "pass"; $error['test']; } if($form->num_errors > 0) { return false; } $this->username = $_SESSION['username'] = $this->userinfo['username']; if($Lremember) { setcookie("cookname", $this->username, time()+COOKIE_EXPIRE, COOKIE_PATH); } return true; } ?> and this is my last part of php code here i check in database for user. <?php require 'mysql.class.php'; $db = &new MySQL(); class Users { function ConfirmLogin($username, $password) { global $db; $select = "SELECT password FROM `sys_members` WHERE login = '$username'"; $result = $db->Query($select); if(!$result || ($db->NumRows($result) < 1)) { return 1; } $dbarray = $db->FetchArray($result); $dbarray['password'] = stripslashes($dbarray['password']); $password = stripslashes($password); if($password == $dbarray['password']) { return 0; } else { return 2; } } ?> can please someone help me i don't know why it doesn't work Quote Link to comment Share on other sites More sharing options...
corbin Posted November 16, 2008 Share Posted November 16, 2008 Whoa. What's even going on? Can we see how you're calling the code? Also, it looks like you're putting things in classes where they don't belong. For example, the login() method shouldn't be in the Sessions class. Logging in has nothing to do with sessions. Sessions are simply what store the state. Quote Link to comment Share on other sites More sharing options...
corbin Posted November 16, 2008 Share Posted November 16, 2008 Eh, I got bored, so I revised/critiqued some of your code: <?php //First off, we'll start with your MySQL class. /* To be honest, it's not very good how it is right now because it's essentially just procedural code with a timer. Also, globals are evil. Just remember that. I can give reasons if you want, but just remember they're evil ;p. Instead, you should use some kind of registry. (The first solution to pop into most people new to OOPs' heads is to use a singleton, but what if you want multiple DB conns?) */ class MySQL { var $totalTime = 0; var $conn = false; //a variable in which to store the connection var $queryCounter = 0; //for some reason you didn't have this here static var $settings = array(); //these will be used for error reporting later. const INVALID_SETTINGS = -4; const NO_CONNECT = -3; const NO_SELECT = -2; const QUERY_ERROR = -1; function MySQL($key) { //this should technically be private, but unfortunately that's not possible in PHP4 //global $config; if(!isset($this->settings[$key])) { throw new Exception("A key was used without settings.", self::INVALID_SETTINGS); } $host = $this->settings[$key]['db_host']; $db_user = $this->settings[$key]['db_user']; $db_pass = $this->settings[$key]['db_password']; $dba = $this->settings[$key]['db_database']; $startTime = $this->getMicroTime(); //you uh.... don't actually have a getMicroTime method, but I'll assume it's in some other code or something that you omitted //mysql_connect($host, $db_user, $db_pass) or die("Could Not Connect To DataBase"); //mysql_select_db($dba) or die("Could Not Select DataBase"); //making the code exit if something goes wrong is just lazy. Also, it would look bad since it wouldn't match the layout of the rest of the site. //I usually use exceptions when something doesn't go as expected. $this->conn = mysql_connect($host, $db_user, $db_pass); if(!$this->conn) throw new Exception(mysql_error($this->conn), self::NO_CONNECT); //create and throw an exception object with a //message of the last mysql error and a code of not connecting if(!mysql_select_db($dba, $this->conn)) throw new Exception(mysql_error($this->conn), self::NO_SELECT); //same as a second ago, except different code $this->totalTime += $this->getMicroTime() - $startTime; } //MySQL Query Command function Query($sql) { $startTime = $this->getMicroTime(); ++$this->queryCounter; if(!$result = mysql_query($sql)) { throw new Exception(mysql_error($this->conn), self::QUERY_ERROR); //do your mail stuff here if you want. However, nothing should be output to the user. Why would a MySQL class handle HTML output? //the exception should be caught later, and then something should be output ;p. } //You shouldn't check mysql_error. You should check if the mysql_query result was false. /*if (mysql_error()) { $this->mailError($sql, mysql_error(), mysql_errno()); return $this->error("<br />There was a SQL error and it has been reported. Sorry for the inconvience. <br />"); } */ $this->totalTime += $this->getMicroTime() - startTime; //I would probably return a result object instead of just a resource.... But, I guess we can do it your way ;p. return $result; } // MySQL Fetch Arrray Query function FetchArray($result) { //$rows = mysql_fetch_array($result); //return $rows; //Why not save a step and just return mysql_fetch_array? return mysql_fetch_array($result); } // mysql_num_rows function NumRows($result) { //$count = mysql_num_rows($result); //return $count; return mysql_num_rows($result); } static function StoreSettings($key, $settings) { $this->settings[$key] = $settings; } function GetInst($key) { //this is our ghetto registry ;p static $insts = array(); if(!isset($insts[$key])) { $insts[$key] = new MySQL($key); } return $insts[$key]; } } //So now, to connect to a server, you would use something like: /* MySQL::StoreSettings("db1", array('db_host' => 'somehost', 'db_user' => 'user', 'db_password' => 'pass', 'db_database' => 'somedb')); //Now, anywhere in your script, you could get the db1 instance of MySQL like this: $db1 = MySQL::GetInst("db1"); $r = $db1->Query("Some query"); */ //Uh, I see what you were trying to do here.... You were trying to make a controller or something, but uhhhhh.... //I'm not sure I even want to mess with this.... x.x /*class Process { function Process() { global $session; if(isset($_POST['login'])) { $this->Login(); } } function Login() { global $session; $cmd = $session->Login($_POST['user'], $_POST[pass], isset($_POST['remember'])); if($cmd) { header ("Location:index.php"); } } }*/ class Session { //this class is entirely pointless, but uh... what ever floats your boat function Session() { $this->time = time(); $this->StartSession(); } function StartSession() { global $users; session_start(); $this->logged_in = $this->CheckLogin(); } //Logging in has nothing to do with sessions.... /* function Login($Luser, $Lpass, $Lremember) { global $users, $form; $field = "user"; // Username Name Field. if(!$Luser || strlen($subuser = trim($Luser)) == 0) { $error['test']; } else { if(!eregi("^([0-9a-z])*$", $Luser)) { $error['test']; } } $field = "pass"; if(!$Lpass) { $error['test']; } if($form->num_errors > 0) { return false; } $Luser = stripslashes($Luser); $result = $users->ConfirmLogin($Luser, md5($Lpass)); if($result == 1) { $field = "user"; $error['test']; } else if($result == 2) { $field = "pass"; $error['test']; } if($form->num_errors > 0) { return false; } $this->username = $_SESSION['username'] = $this->userinfo['username']; if($Lremember) { setcookie("cookname", $this->username, time()+COOKIE_EXPIRE, COOKIE_PATH); } return true; } */ } class Users { const USER_NOT_EXIST = -2; const WRONG_PASSWORD = -1; const SUCCESS = 0; //Eh, this function will work.... I'll just mod it a little function ConfirmLogin($username, $password) { $db = MySQL::GetInst("db1"); $select = "SELECT password FROM `sys_members` WHERE login = '$username'"; $result = $db->Query($select); //!result not needed any more since it would throw an exception if(!$db->NumRows($result)) { return self::USER_NOT_EXIST; } $dbarray = $db->FetchArray($result); //something is wrong if you're having to strip the slashes from your DB password.... x.x $dbarray['password'] = stripslashes($dbarray['password']); //Are you using magic quotes? OMG EWWWWWWW.... $password = stripslashes($password); if($password == $dbarray['password']) { return self::SUCCESS; } else { return self::WRONG_PASSWORD; } } } //So now a little example of your classes: try { session_start(); $logged = isset($_SESSION['username']); if(!$logged) { if($_POST) { MySQL::StoreSettings("db1", array(/*db settings*/)); $user = (isset($_POST['username'])) ? $_POST['username'] : ''; $pass = (isset($_POST['password'])) ? $_POST['password'] : ''; $u = new User(); $r = $u->ConfirmLogin($user, $pass); if($r == User::SUCCESS) { $_SESSION['username'] = $user; } else { echo '<div style="color: red;">Incorrect username or password.</div>'; } } if(!$logged) { echo <<<FORM <form action="" method="POST"> Username: <input type="text" name="username" value="" /><br /> Password: <input type="password" name="password" value="" /><br /> <input type="submit" value="Login!" /><br /> </form> FORM; } } if($logged) { //This is actually not the best way to check if someone is logged in or not, especially in combination with your classes, //but it will work for this example echo "Hello {$_SESSION['username']}, you are currently logged in."; } } catch (Exception $e) { echo "There was an exception:<br />"; var_dump($e); } Quote Link to comment Share on other sites More sharing options...
burnside Posted November 17, 2008 Share Posted November 17, 2008 @ CORBIN some nice comments there made me giggle Quote Link to comment Share on other sites More sharing options...
corbin Posted November 17, 2008 Share Posted November 17, 2008 //this is our ghetto registry ;p Was my favorite. Quote Link to comment Share on other sites More sharing options...
burnside Posted November 18, 2008 Share Posted November 18, 2008 //this is our ghetto registry ;p Was my favorite. LMAO yup but uh... what ever floats your boat //Are you using magic quotes? OMG EWWWWWWW.... i liked that as well. Quote Link to comment Share on other sites More sharing options...
corbin Posted November 18, 2008 Share Posted November 18, 2008 Eh, life would be boring without saying random things ;p. Quote Link to comment Share on other sites More sharing options...
DarkWater Posted November 18, 2008 Share Posted November 18, 2008 I don't think that the OP has any idea of what OOP is really used for. OOP should not just be wrappers for a bunch of completely unrelated procedural code. OOP is used to create meaningful, individual structures that accomplish a certain task. Read 448191's (did I get that right? >_<) tutorials on OOP on the main page, starting with Part 1. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.