best way to clean up code

[webguync]

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?

[coupe-r]

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

[webguync]

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.

[coupe-r]

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

[webguync]

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 

[ignace]

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.

[Ken2k7]

And LOGIN_AMBIGUITY is not defined.