SitePoint Sponsor

User Tag List

Results 1 to 6 of 6
  1. #1
    SitePoint Member
    Join Date
    Apr 2001
    Location
    Norway
    Posts
    0
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Optimize objects/classes in login form

    Hi

    I am pretty new to object orientation, but hoped you could help me optimize this login code. I am a bit unsure what to put in classes and what to have in main. Here I have tried getting all response in the login.php...

    Any thoughts are highly welcome!

    You may also see that I create db object from another class...is that allowed, normally ?

    LOGIN.PHP
    PHP Code:
    <? session_start(); ?>
    <html>
    <?
    include_once("classes/db.php" );
    include_once(
    "classes/FormFactory.php" );
    include_once(
    "classes/User.php" );
    $form = new FormFactory();
    $user = new User();
    settype($username"string" );
    settype($password"string" );
    settype($submit"string" );
    settype($url,"string" );
    include_once(
    "language/eng.inc" );
    $url URL;
    if (isset(
    $_GET["username"]))
    $username $_GET["username"];
    if (isset(
    $_GET["submit"]))
    {
    $username $_GET["username"];
    $password $_GET["password"];
    $submit $_GET["submit"];
     
    if (
    $user->verify($username,$password))
    {
    echo 
    LOGGED_IN;
     
    }
    else 
    {
    echo 
    NOT_FOUND "<br />";
    echo 
    FORGOT_PASSWD;
     
    }
     
    }
    else 
    {
    echo 
    "<p>";
    echo 
    "<b>" WELCOME "</b><br />";
     
    echo 
    $form->formHeader("post","login.php" );
    echo 
    USERNAME "<br />";
    echo 
    $form->createTextField("username","$username",12,20);
    echo 
    "<p />";
     
    echo 
    PASSWORD "<br />";
    echo 
    $form->createTextField("password","$password",12,20);
    echo 
    "<br />";
     
    $LOGIN=LOGIN;
    echo 
    $form->createSubmitButton("submit",$LOGIN);
    echo 
    $form->formFooter();
     
    echo 
    FORGOT_PASSWD;
    echo 
    "</p>";
    }
    ?>
    </html>
    Snip from class User.PHP
    PHP Code:
     
    function verify ($username,$password)
        {
         
    $this->username $username;
         
    $this->password $password;
     
         
    $db = new DB();
         
    $sql "SELECT username FROM users WHERE username = '$this->username' AND password ='$this->password'";
         
    $res=$db->Q($sql);
     
         if (!
    $db->NumRows($res))
            {
             return 
    false;
            }
            else
            {
             
    session_register("username" );
             return 
    true;         
            }
     
        } 
    Last edited by are; Mar 14, 2003 at 18:10.

  2. #2
    As the name suggests... trickie's Avatar
    Join Date
    Jul 2002
    Location
    Melbourne, Australia
    Posts
    678
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    A few suggestions...

    - Don't create the DB instance inside the user class. Instead, in my opinion, it is better to have one DB object instantiated in the global space, and then passed to and used by any other classes. That means that there is only ever one instance of the DB class (Singleton Pattern), and you don't hard code what class names or types you are using. So instantiate the DB and then pass it as an argument when you instantiate the user class, or even just pass it as a parameter in the Users->verify method. eg:
    PHP Code:
    <?php
    $db 
    =& new DB(); // this class can then be changed and you don't break everything else.
    $user =& new Users($db);
    ?>
    - Use a DB result class. At the moment you are restricting yourself to one DB (maybe you will only ever use one) by not abstracting the DB result. Have a look at Voostind 's Eclipse package (the link is in the Advanced PHP resources thread, at the top of the forum) and see how he has done it.

    - I can see that you are trying to check the form variables and force them to be strings, but that will be done by PHP anyway, and isn't necessary. Neither is:
    PHP Code:
    <?php
    $username 
    $_POST['username'];
    ?>
    because the super-globals are reasonably secure and unless you need to set that for another reason just use $_POST['username'] where ever you would use username. They are one and the same.

  3. #3
    public static void brain Gybbyl's Avatar
    Join Date
    Jun 2002
    Location
    Montana, USA
    Posts
    647
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    but still check for $this->, because you can't do that in PHP5 anymore.
    I'm still using it, and it's working fine. What am I supposed to be using now? a '.'? (Or should I just call it Japha (java + php)?)
    Ryan

  4. #4
    SitePoint Enthusiast BDKR's Avatar
    Join Date
    Sep 2002
    Location
    Clearwater, Florida
    Posts
    69
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Just an extension of what Trickie said for the most part. Another option to consider is to create seperate db connection and query classes. In this way you could maintain multiple queries against a database(s) via one connection to the db. Also, if you need to do queries back to the database based on the results of a previous query, it's much more effecient to do it in this manner.

    From an RDD (Responsibility Driven Design) point of view, this is a breaking down of one uber database class into two seperate areas of responsibility. One for the maintenance of the connection and another for handling queries. The implication here is that the db query object has knowledge of the connection object, or at least it's connection status.

    Cheers,
    BDKR
    If you're not on the gas, you're off the gas!

  5. #5
    SitePoint Member
    Join Date
    Apr 2001
    Location
    Norway
    Posts
    0
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    - I can see that you are trying to check the form variables and force them to be strings, but that will be done by PHP anyway, and isn't necessary.

    Hi

    Thanks for all advice, from both of you. The reason for using settype is that else PHP gives Notice about it. I want to avoid that as much as possible. I know this can be set in PHP.ini, so that Notices donīt appear, but I have my reasons to keep avay for notices also.
    Regards,
    Are

  6. #6
    SitePoint Enthusiast BDKR's Avatar
    Join Date
    Sep 2002
    Location
    Clearwater, Florida
    Posts
    69
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by are
    Hi
    The reason for using settype is that else PHP gives Notice about it. I want to avoid that as much as possible. I know this can be set in PHP.ini, so that Notices donīt appear, but I have my reasons to keep avay for notices also.
    I agree with you for not wanting the notices. We could allways just suppress the notices, but the php engine still has to do something with the cause of these notices and that slows down execution of your script.

    As for using settype, you may be better off to do something like...

    PHP Code:
    <?
    $username
    ='';
    $password='';
    $submit='';
    $url='';
    ?>
    Ultimately, what you're doing here is setting the type of variables that don't yet exist (based on what I can see of your script). The engine it seems is creating the vars for you, but it's much more work to do it in this manner than what I described above. At least at this rate, you won't have the engine crying about trying to perform operations against vars that don't yet exist. And lastly, if you assign a value that is of type string, the php engine is more than likely going to set it's type correclty.

    More often than not, people get notices just for this reason. Trying to test the condition (or something) of a var that doesn't yet exist. Even more people don't get these notices becuase the error reporting is set too such a level that they never seen. Therefore, notices from sloppy code like this...

    PHP Code:
    <?
    ++$array[element];
    ?>
    may never be noticed until it bites you in the tail. I know from experience. :-)

    BUT, why not just use the $_GET and $_POST vars everywhere? Aren't you needlessly increasing your workload by doing ...

    PHP Code:
    <?
    $username 
    $_GET["username"];
    $password $_GET["password"];
    $submit $_GET["submit"];
    ?>
    ... not to mention doubling the amount of needed resources for these vars? On the next line, why can't you just do this?

    PHP Code:
    <?
    if ($user->verify($_GET['username'], $_GET['password']))
       { 
    /* yada yada yada */ }
    ?>
    I know it's cumbersome, but with a good editor you could create macros to do this for you. ( :-) ) You also decrease the overhead and execution time of the script.

    Cheers,
    BDKR
    If you're not on the gas, you're off the gas!


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
  •