RopeADope Posted June 25, 2010 Share Posted June 25, 2010 Well...I've read the tutorials and did my best but I still feel I've made a terrible mess of what OOPHP should be. Due to lack of experience, I'm not entirely sure this makes sense but it does in my head so here it goes.... class auth { var $username; var $password; function connect(){ $db="main_db"; $dbuser="db_user"; $dbpass="db_password"; $host="localhost"; mysql_connect($host, $dbuser, $dbpass)or die('Error establishing connection!'); mysql_select_db($dbname)or die('Error connecting to the database!'); } function clean_input($username,$password){ $regex="'/^[A-Za-z0-9]+$/'"; //Expression to match $repex=""; //Replace unmet characters with nothing, i.e. remove them from the string preg_replace($regex, $repex, $username); preg_replace($regex, $repex, $password); } function auth_user(){ $sql=mysql_query("SELECT * FROM users WHERE username='$username' AND password='$password'"); $result=mysql_result($sql); if($result==1) { header('Location:main.php'); }else{ header('Location:index.php'); }; }; }; Sneaking suspicions: 1) I should have a setter method somewhere. 2) The connect function should take arguments 3) clean_input function is off somehow...RegEx off?(goal is to only allow alphanumeric characters) Mainly looking for ways to improve efficieny, make it more reusable, and more importantly that it works. Thanks in advance! Quote Link to comment https://forums.phpfreaks.com/topic/205851-first-attempt-at-oophp-auth-class/ Share on other sites More sharing options...
trq Posted June 25, 2010 Share Posted June 25, 2010 Cleaning user input & making a database connection has nothing at all to do with user authentication and should be in there own classes. Your auth_user() method is completely floored. mysql_result() returns either a result resource if the query succeeds (this is not the same as finding a result) or false otherwise. It will never return 1. Quote Link to comment https://forums.phpfreaks.com/topic/205851-first-attempt-at-oophp-auth-class/#findComment-1077174 Share on other sites More sharing options...
RopeADope Posted June 25, 2010 Author Share Posted June 25, 2010 Cleaning user input & making a database connection has nothing at all to do with user authentication and should be in there own classes. Your auth_user() method is completely floored. mysql_result() returns either a result resource if the query succeeds (this is not the same as finding a result) or false otherwise. It will never return 1. Gotcha. I'll revise and re-post. Quote Link to comment https://forums.phpfreaks.com/topic/205851-first-attempt-at-oophp-auth-class/#findComment-1077179 Share on other sites More sharing options...
RopeADope Posted June 25, 2010 Author Share Posted June 25, 2010 Ok not sure if I made it better or worse but here it is. <?php class auth { var $username; var $password; function auth_user($username,$password){ $sql=mysql_query("SELECT * FROM users WHERE username='$username' AND password='$password'"); $result=mysql_result($sql); $num=mysql_num_rows($result); if($num==1) { header('Location:main.php'); }else{ header('Location:index.php'); }; }; }; class connect{ $host="localhost"; $dbuser="main_user"; $dbpass="password"; $dbname="main_db"; mysql_connect($host, $dbuser, $dbpass)or die('Error establishing connection!'); mysql_select_db($dbname)or die('Error connecting to the database!'); } class clean_input{ $regex="'/^[A-Za-z0-9]+$/'"; //Expression to match $repex=""; //Replace unmet characters with nothing, i.e. remove them from the string preg_replace($regex, $repex, $username); preg_replace($regex, $repex, $password); } ?> Quote Link to comment https://forums.phpfreaks.com/topic/205851-first-attempt-at-oophp-auth-class/#findComment-1077194 Share on other sites More sharing options...
trq Posted June 25, 2010 Share Posted June 25, 2010 You might want to at least get a grasp of functional programming before eve attempting OOP. The classes connect() & clean_input syntactically incorrect and will not function. auth_user() should probably just return true or false. Making it redirect to specific pages makes it completely tied to the current application. Quote Link to comment https://forums.phpfreaks.com/topic/205851-first-attempt-at-oophp-auth-class/#findComment-1077205 Share on other sites More sharing options...
RopeADope Posted June 25, 2010 Author Share Posted June 25, 2010 You might want to at least get a grasp of functional programming before eve attempting OOP. The classes connect() & clean_input syntactically incorrect and will not function. auth_user() should probably just return true or false. Making it redirect to specific pages makes it completely tied to the current application. Yeah I'm realizing that now...I need to start simpler. I understand the concept(I think). A class is a blueprint that has properties and the methods work with those properties(right?). I'm going to give it another go but make it much simpler... Quote Link to comment https://forums.phpfreaks.com/topic/205851-first-attempt-at-oophp-auth-class/#findComment-1077209 Share on other sites More sharing options...
RopeADope Posted June 25, 2010 Author Share Posted June 25, 2010 Ok here's my simplified task...connect and select a db. My thought is that connect and select should be in the same class because you wouldn't need to connect to MySQL and not do anything, so selecting a db would be inherent. With that said, should the connect and select functions be combined into one? <?php class connect { var $host; var $dbuser; var $dbpass; var $db; //Set variables for db connection function connect($host,$dbuser,$dbpass) { $this->host=$host; $this->dbuser=$dbuser; $this->dbpass=$dbpass; mysql_connect($host,$dbuser,$dbpass)or die('No connection'); } function select($db) { $this->db=$db; mysql_select_db($db)or die('No database selected'); } } ?> Quote Link to comment https://forums.phpfreaks.com/topic/205851-first-attempt-at-oophp-auth-class/#findComment-1077212 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.