SitePoint Sponsor

User Tag List

Results 1 to 9 of 9
  1. #1
    gimme the uuuuuuuuuuu duuudie's Avatar
    Join Date
    Feb 2004
    Location
    Switzerland
    Posts
    2,253
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    need help and advice with this confirmRegistration class.

    Hi

    I will probably have to refactor this class anyways, even though I could make it work (which is unfortunately not the case yet) so I thought it would be a good idea to get some advice from experienced PHP programmers

    The purpose of this class is to confirm members registration. An email is sent to the user. He has to click on a link containing a 32 chars string with his userID. If both matches, The user is registered in the 'users' table and deleted from the 'userstemp' table.

    I really would like to improve my actual code to make it as efficient as possible.

    Here is the class:

    PHP Code:

    class ConfirmRegistration {

    var 
    $db;

    var 
    $confirmCode;

    var 
    $confirm;

    var 
    $userID;

    var 
    $sql;

        function 
    ConfirmRegistration(&$db$confirmCode$userID) {
        
            
    $this->db = &$db;
            
            if ( ( isset(
    $confirmCode) ) && ( isset($userID) ) ) {
                
    $this->confirmCode $this->clean($confirmCode);
                
    $this->userID $this->clean($userID);
                
    $this->accessConfirm();
                return 
    true;
            }
            else {
                return 
    false;
            }
        }
        
        function 
    accessConfirm() {
            
    $sql=
            
    "SELECT userID
            , username
            , password
            , permission
            , registrationDate
            , lastLogin
            , uniqueID
            FROM userstemp
            WHERE
            confirmCode = '
    $this->confirmCode'
            AND userID = 
    $this->userID";
            
    $this->confirm $this->db->query($sql);
            if ( 
    $this->confirm->size() == ) {
                
    $this->addNewUser();
                return 
    true;
            }else {
                return 
    false;
            }
        }
                    
        function 
    addNewUser() {

            
    $row $this->confirm->fetch();
            
    $username=$row['username'];
            
    $password=$row['password'];
            
    $permission=$row['permission'];
            
    $registrationDate=$row['registrationDate'];
            
    $lastLogin=$row['lastLogin'];
            
    $uniqueID=$row['uniqueID'];
                        
            
    $insert=
            
    "INSERT INTO users
            SET
            username = '
    $username'
            , password = '
    $password'
            , permission = 
    $permission
            , registrationDate = 
    $registrationDate
            , lastLogin = 
    $lastLogin
            , uniqueID = '
    $uniqueID'";
            
    $this->db->query($insert);
            
    $this->deleteTempAccount();
        }
        
        function 
    deleteTempAccount() {    
            
    $delete=
            
    "DELETE FROM userstemp
            WHERE userID = 
    $this->userID";
                        
            
    $this->db->query($delete);
        }
        
        function 
    clean($string) { 
            
    $cleaned trim($string); 
            
    $cleaned strip_tags($cleaned);
            
    $cleaned addslashes($cleaned);
            
            return 
    $cleaned;
        }

    and here is the 'client' part. As you might notice, I have to perform a few checks to display the relevant message. Here again, I need some advice on how to do it.
    PHP Code:
    $db=& new MySQL($host,$dbUser,$dbPass,$dbName);
    $confirm=& new ConfirmRegistration($db, $_GET['confirmCode'], $_GET['$userID']);

                        Congratulations. You have successfully set up your account.
                        <br />
                        Please login to your <a href="<?=$url?>userCP/userCP_login.php">userCP</a>.
                        <?php
                        
    if ( ( !$confirm->accessConfirm() ) ) {
                        
    ?>
                        We are sorry but it seems that a problem occured during your account confirmation.
                        <br />
                        Please contact a staff member for more info.
                        <?php
                        
    }
                        
    ?>
    thanks a lot for your time and your energy

  2. #2
    Non-Member
    Join Date
    Jan 2004
    Location
    Planet Earth
    Posts
    1,764
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hello

    I would seperate the desicion process from the other functions of this class, which should basically follow some kind of Gateway Pattern

    For example, this part should be a seperate class, below

    PHP Code:
    ...
    if ( ( isset(
    $confirmCode) ) && ( isset($userID) ) ) { 
                
    $this->confirmCode $this->clean($confirmCode); 
                
    $this->userID $this->clean($userID); 
                
    $this->accessConfirm(); 
                return 
    true
            } 
            else { 
                return 
    false
            }
    ... 
    Also, the IF condition doesn't belong in the constructor in my view.

    Hope this helps

  3. #3
    Non-Member
    Join Date
    Jan 2004
    Location
    Planet Earth
    Posts
    1,764
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    PHP Code:
    function addNewUser() { 

            
    $row $this->confirm->fetch(); 
            
    $username=$row['username']; 
            
    $password=$row['password']; 
            
    $permission=$row['permission']; 
            
    $registrationDate=$row['registrationDate']; 
            
    $lastLogin=$row['lastLogin']; 
            
    $uniqueID=$row['uniqueID']; 
    ... 
    These variables would be better passed to the class method as parameters as well ?? Likewise, below

    PHP Code:
    function deleteTempAccount() {     
            
    $delete
            
    "DELETE FROM userstemp 
            WHERE userID = 
    $this->userID";
    ... 
    Could be

    PHP Code:
    function deleteTempAccount$id ) {     
            
    $delete
            
    "DELETE FROM userstemp 
            WHERE userID = '"
    .$id."'"
    ... 
    For example.

    The way you have done it at the moment, there is a direct (unrequired) relationship between the user and this class method, which in fact is an Account of an user, but has nothing it's self (as an entity) to do with a user directly, to me anyways.

    Passing the user id as a parameter also gives you a lot more flexibility on how you are able to use the class method in regards to where the user id comes from ?

    Just my thoughts though

  4. #4
    SitePoint Enthusiast
    Join Date
    Jan 2003
    Posts
    91
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Another thing I would do would be to take the ConfirmRegistration::clean() method out of the class and move it into some utility class so that:

    1) It can be re-used by other scripts
    2) You will have a central place for this method so that should you ever decide to change or improve it, the changes will flow through to everywhere it is used

    eg.

    PHP Code:
     /*
         Form Input Utility Functions
         Call this class whatever you want!
     */
     
     
    class InputCleaning
     
    {
     
         function 
    clean($string) {
             
    $cleaned trim($string);
             
    $cleaned strip_tags($cleaned);
             
    $cleaned addslashes($cleaned);
             
             return 
    $cleaned;
         }
     
     } 
    Then you can just include this class and call the clean method statically. eg.

    PHP Code:
     $value InputCleaning::clean($value); 
    Then in your ConfirmRegistration class

    PHP Code:
     class ConfirmRegistration {
     
     var 
    $db;
     
     var 
    $confirmCode;
     
     var 
    $confirm;
     
     var 
    $userID;
     
     var 
    $sql;
     
         function 
    ConfirmRegistration(&$db$confirmCode$userID) {
         
             
    /** INCLUDE THE UTILITY FUNCTIONS CLASS **/
             
    include( "utilities.inc.php" );
             
    $this->db = &$db;
             
             if ( ( isset(
    $confirmCode) ) && ( isset($userID) ) ) {
                 
    $this->confirmCode InputCleaning::clean($confirmCode);
                 
    $this->userID InputCleaning::clean($userID);
                 
    $this->accessConfirm();
                 return 
    true;
             }
             else {
                 return 
    false;
             }
         }
         
         function 
    accessConfirm() {
             
    $sql=
             
    "SELECT userID
             , username
             , password
             , permission
             , registrationDate
             , lastLogin
             , uniqueID
             FROM userstemp
             WHERE
             confirmCode = '
    $this->confirmCode'
             AND userID = 
    $this->userID";
             
    $this->confirm $this->db->query($sql);
             if ( 
    $this->confirm->size() == ) {
                 
    $this->addNewUser();
                 return 
    true;
             }else {
                 return 
    false;
             }
         }
                     
         function 
    addNewUser() {
     
             
    $row $this->confirm->fetch();
             
    $username=$row['username'];
             
    $password=$row['password'];
             
    $permission=$row['permission'];
             
    $registrationDate=$row['registrationDate'];
             
    $lastLogin=$row['lastLogin'];
             
    $uniqueID=$row['uniqueID'];
                         
             
    $insert=
             
    "INSERT INTO users
             SET
             username = '
    $username'
             , password = '
    $password'
             , permission = 
    $permission
             , registrationDate = 
    $registrationDate
             , lastLogin = 
    $lastLogin
             , uniqueID = '
    $uniqueID'";
             
    $this->db->query($insert);
             
    $this->deleteTempAccount();
         }
         
         function 
    deleteTempAccount() {    
             
    $delete=
             
    "DELETE FROM userstemp
             WHERE userID = 
    $this->userID";
                         
             
    $this->db->query($delete);
         }
         
     
     } 

  5. #5
    gimme the uuuuuuuuuuu duuudie's Avatar
    Join Date
    Feb 2004
    Location
    Switzerland
    Posts
    2,253
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    thanks a lot WM

    I will work on that and show the new version in the near future.

    What about it? What is the best approach for such tasks?
    PHP Code:
    <?php 
                        
    if ( ( !$confirm->accessConfirm() ) ) { 
                        
    ?> 
                        We are sorry but it seems that a problem occured during your account confirmation. 
                        <br /> 
                        Please contact a staff member for more info. 
                        <?php 
                        

                        
    ?>
    thanks for your time

  6. #6
    Non-Member
    Join Date
    Jan 2004
    Location
    Planet Earth
    Posts
    1,764
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    PHP Code:
    if ( ( isset($confirmCode) ) && ( isset($userID) ) ) { 
    What I do is to do the validation of variables going to a class prior to sending them, so these variables would already be valid before use

    Usually by the client script is where the IF condition belongs, as I see it ?

  7. #7
    gimme the uuuuuuuuuuu duuudie's Avatar
    Join Date
    Feb 2004
    Location
    Switzerland
    Posts
    2,253
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Actually my question is about:

    if a function returns true {
    display stuff
    }else{
    display other stuff
    }

  8. #8
    Non-Member
    Join Date
    Jan 2004
    Location
    Planet Earth
    Posts
    1,764
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Oh !! In that case, then yes, that part is fine enough

  9. #9
    gimme the uuuuuuuuuuu duuudie's Avatar
    Join Date
    Feb 2004
    Location
    Switzerland
    Posts
    2,253
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    k thanks


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •