Optimize objects/classes in login form

**are** · Mar 14, 2003 19:05

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;         
        }
 
    }

**trickie** · Mar 14, 2003 19:50

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.

**Gybbyl** · Mar 15, 2003 01:52

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

**BDKR** · Mar 15, 2003 11:18

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

**are** · Mar 17, 2003 10:22

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

**BDKR** · Mar 17, 2003 11:28

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

User Tag List

Thread: Optimize objects/classes in login form

Thread Tools

Display

Optimize objects/classes in login form

Bookmarks

Bookmarks

Posting Permissions