webguync Posted April 27, 2010 Share Posted April 27, 2010 I have a page which uses several different Queries and 1 it's hard to follow and two the Queries might be interfering with one another. Is it best to use functions or includes to clean up the code? Here is an example. $query = "SELECT username,pwid,name,user_id FROM test_roster_April2010 WHERE pwid = '$pwid' AND username='$username'"; $result = mysql_query($query) or die(mysql_error()); if(mysql_num_rows($result) == 0) { // Gives an error if the username/pw given does not exist. // or if something else is wrong. echo "<h2 class='fail'>You have entered a username or password that does not match our database records. please try again.<br><br>You will be redirected back to the login screen in five seconds.</h2> " . mysql_error(); echo "<meta http-equiv='refresh' content='5; url=StudentLogin.php'>"; exit(); } else { $row = mysql_fetch_object($result); $_SESSION['name'] = $row->name; $_SESSION['user_id'] = $row->user_id; $_SESSION['username'] = $username; $_SESSION['sid'] = session_id(); $_SESSION['ip'] = $_SERVER['REMOTE_ADDR']; $user_id = $_SESSION['user_id']; print_r($_SESSION); could I put all of that into a function? Quote Link to comment Share on other sites More sharing options...
coupe-r Posted April 27, 2010 Share Posted April 27, 2010 This code seems like a one and done. Creating a function would be for common code you will always use and instead of adding it to all pages, create a function and use a simple call. Do you just hate having a big scroll bar when looking at your code?? Quote Link to comment Share on other sites More sharing options...
webguync Posted April 27, 2010 Author Share Posted April 27, 2010 Yea, and it an get confusing with all of the Queries up and down the page. Thought it might be easier to debug with functions, but all of the queries are unique to one page. Quote Link to comment Share on other sites More sharing options...
coupe-r Posted April 27, 2010 Share Posted April 27, 2010 The way you have it looks good to me, especially if you only have 1 query per page. If you made functions, you would have a bunch of functions and function names in 1 php file that you would have to remember. Just tab everything out and make it look nice... Quote Link to comment Share on other sites More sharing options...
webguync Posted April 27, 2010 Author Share Posted April 27, 2010 oh, don't have one query per page, have about 6 of em, and they are all performing something different, that is what makes it confusing Quote Link to comment Share on other sites More sharing options...
ignace Posted April 27, 2010 Share Posted April 27, 2010 Yes, you can put that in a function. However while you are "refactoring" your code you could do it with flexibility in mind I for example would re-factor the above code like: function cleanup($value) {/*implementation comes here*/} function validUsername($username) {} function validPassword($password) {} define('LOGIN_OK', 1); define('LOGIN_EMPTY', 2); define('LOGIN_INVALID', 4); define('LOGIN_UNKNOWN', ; define('LOGIN_AMBIGUITY', 16); function login($username, $password) { if (empty($username) || empty($password)) return LOGIN_EMPTY; $username = clean($username); $password = clean($password); if (!validUsername($username) || !validPassword($password)) return LOGIN_INVALID; $query = "SELECT user_id, name FROM test_roster_April2010 WHERE username = '$username' AND pwid = '$password'"; $result = mysql_query($query); if ($result !== false && ($num_rows = mysql_num_rows($result)) === 0) return LOGIN_UNKNOWN; if ($num_rows > 1) return LOGIN_AMBIGUITY; $_SESSION = array_merge($_SESSION, mysql_fetch_assoc($result)); return LOGIN_OK; } //YOUR CODE REFACTORED: // //$response = login($_POST['username'], $_POST['pwid']); //if ($response === LOGIN_INVALID) { // echo '<h2 class="fail">You have entered a username or password that does not match our database records.', // ' please try again.<br><br>You will be redirected back to the login screen in five seconds.</h2>', // '<meta http-equiv="refresh" content="5; url=StudentLogin.php">'; // exit(0); //} However for the best result it would be best to use classes as your login implementation may vary depending on the project (OpenID, FacebookConnect, ..). More so because your query is hard-coded, not in every project will your table be called test_roster_April2010 nor will it always contain the fields username, pwid, userid, name as these are project specific. Quote Link to comment Share on other sites More sharing options...
Ken2k7 Posted April 27, 2010 Share Posted April 27, 2010 And LOGIN_AMBIGUITY is not defined. 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.