Page 1 of 1

My first script

Posted: Fri Sep 22, 2006 2:28 am
by draco2317
This is the first (and so far only) php script i have written. I was wanting to know how badly I did :)

Code: Select all

<?php
include("include/dbconnect.php");
include("include/settings.php");

$recover=mysql_query("SELECT name, user, pass FROM models WHERE email = '$email'");
$subject='Log In Info';

$row = mysql_fetch_assoc($recover);
if ($row == "")
{
echo "The email address you submitted was not found in our database.  Check to make sure you entered in the address in correctly.";
}
else 
{ 
$mesaj = "From time to time people forget many things, dont feel bad. Your information is listed below.\n\nUsername: $row[user]\nPassword: $row[pass]";
mail($email, $subject , $mesaj ,"From: webmaster@reddragonmodels.net\nReply-To: webmaster@reddragonmodels.net\n");
}
?>

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>

<body>
<?php include("header.php"); ?>
<?php print "$row[name],\n"; ?><br><br>
   <font size="+1"><b>Your Username and password has been sent<br> to the email account your registered with.</b></font><br>
</body>
</html>

Posted: Fri Sep 22, 2006 2:47 am
by Christopher
Looks ok to me. A couple of things:

- Why is settings included before dbconnect? It seem like the DB connection info could be in settings to centralize configuration data.

- You might want to add error checking in case the query fails.

- If no row is found you echo an error message. Better would be to assign the value to a string and display it below in the <body>. The same goes for the message if a row is found -- assign the text that is currently at the bottom within the if. Then the HTML section at the bottom just displays one string or another.

Posted: Fri Sep 22, 2006 2:57 am
by draco2317
- Why is settings included before dbconnect? It seem like the DB connection info could be in settings to centralize configuration data.
Honestly I dont know. The script i wrote is based off one i bought. The designer forgot to write that page, and said it would be a wihle before we could do so, so i wrote it myself. The includes were always laid out that way
- You might want to add error checking in case the query fails.
I completely missed that, after reading that i tested it by submitting empty variables and it acted as if everything was okay. Thanks for pointing that out.

Posted: Thu Nov 02, 2006 12:51 pm
by wyrmmage
looks pretty good, although I would point out a couple of things.
First: why are you switching back and forth between \n and <BR>?
Secondly: most people use echo"" instead of print"". print"" returns a value saying whether whatever it was priting to the screen actually got there and, since most people don't normally check to see if a string got put on the screen or not, it isn't used much. Most people use echo"" or, in my case, I use echo(''); If you use echo, you must do something like this:

Code: Select all

<?
$x = 1;
echo('$x is this: ' . $x . '<BR>');
?>
instead of going:

Code: Select all

<?
$x = 1;
echo"the variable x is this: $x <Br>";
?>
Using echo('') may seem awkward and not terribly useful at first, but I prefer it, mostly because then I don't have to escape double quotes, which is what I use in HTML tags ;)

-wyrmmage