Newbie - Someone suggest ways to improve my code
Moderator: General Moderators
Newbie - Someone suggest ways to improve my code
Hi all,
I am very new to PHP, but I am finding allsorts of great info, tutorials and advice from the internet.
Would people be willing to overlook this code I have written and suggest ways in which to improve my code. Or even suggest resources for writing better PHP?
It takes a form input, called suggestion and then e-mails it off. I know it is very basic.
<?php
if ("$_POST[Suggestion]" == "")
{
echo "<center><FONT SIZE=2 Face=verdana>";
echo "Sorry you have not entered a suggestion<br>";
echo "Please use your browsers back button.<br>";
echo "</FONT></center>";
}
else
{
echo "<center><FONT SIZE=2 Face=verdana>";
echo "Thanks you entered the following suggestion<br>";
echo "$_POST[Suggestion]";
echo "</FONT></center>";
/* recipients */
$to = "myaddress@somewhere.com";
/* suggestion */
$subject = "Good ideas submission";
/* message */
$message = "$_POST[Suggestion]";
/* and now mail it */
mail($to, $subject, $message);
}
?>
Cheers
James
I am very new to PHP, but I am finding allsorts of great info, tutorials and advice from the internet.
Would people be willing to overlook this code I have written and suggest ways in which to improve my code. Or even suggest resources for writing better PHP?
It takes a form input, called suggestion and then e-mails it off. I know it is very basic.
<?php
if ("$_POST[Suggestion]" == "")
{
echo "<center><FONT SIZE=2 Face=verdana>";
echo "Sorry you have not entered a suggestion<br>";
echo "Please use your browsers back button.<br>";
echo "</FONT></center>";
}
else
{
echo "<center><FONT SIZE=2 Face=verdana>";
echo "Thanks you entered the following suggestion<br>";
echo "$_POST[Suggestion]";
echo "</FONT></center>";
/* recipients */
$to = "myaddress@somewhere.com";
/* suggestion */
$subject = "Good ideas submission";
/* message */
$message = "$_POST[Suggestion]";
/* and now mail it */
mail($to, $subject, $message);
}
?>
Cheers
James
Looks good to me. There are always ways to improve code or do it differently. Everyone believes their way is best!
If the suggestion is empty, the user is returned to the previous page using the referer. However, you may want to do it the way you did, because you at least have an error message.
Remove the quotes from the $_POST variable, otherwise PHP will treat it as a string. The quotes need to go around the array index Suggestion. I have also placed the block of HTML outside PHP because this does not need parsing by the PHP engine, saving a bit of time (Makes a difference on giant pages!)
Otherwise, looks good.
Code: Select all
<?php
if ($_POSTї"Suggestion"] == "")
{
$referer = $_SERVERї"HTTP_REFERER"]; // Could be $_ENV
header("Location: " .$referer);
}
else
{
?>
<center><FONT SIZE=2 Face=verdana>Thanks you entered the following suggestion<br>
<?php
echo $_POSTї"Suggestion"];
?>
</FONT></center>
<?php
/* recipients */
$to = "myaddress@somewhere.com";
/* suggestion */
$subject = "Good ideas submission";
/* message */
$message = $_POSTї"Suggestion"];
/* and now mail it */
mail($to, $subject, $message);
}
?>Remove the quotes from the $_POST variable, otherwise PHP will treat it as a string. The quotes need to go around the array index Suggestion. I have also placed the block of HTML outside PHP because this does not need parsing by the PHP engine, saving a bit of time (Makes a difference on giant pages!)
Otherwise, looks good.
Great. Thanks for the quotation usage tip.
I am now hoping to add a name field.
The field is optional, so I want the $message variable to contain the persons name if it is entered, and obiously not if it is not.
I tried the following changes in my code, at /* message */ section. For some reason I get a parse error, any idea why? Plus is there some way of specifying a carriage return in the message field?
Notice: Undefined index: Name in c:\inetpub\wwwroot\php\tellus.php on line 125
I am now hoping to add a name field.
The field is optional, so I want the $message variable to contain the persons name if it is entered, and obiously not if it is not.
I tried the following changes in my code, at /* message */ section. For some reason I get a parse error, any idea why? Plus is there some way of specifying a carriage return in the message field?
Notice: Undefined index: Name in c:\inetpub\wwwroot\php\tellus.php on line 125
Code: Select all
<?php
if ($_POSTї"Suggestion"] == "")
{
?>
<center><FONT SIZE=2 Face=verdana>
<p>Sorry you have not entered a suggestion</p>
<p>Please use your browsers back button.</p>
</FONT></center>
<?php
}
else
{
?>
<center><FONT SIZE=2 Face=verdana>Thanks you entered the following suggestion<br>
<?php
echo $_POSTї"Suggestion"];
?>
</FONT></center>
<?php
/* recipients */
$to = "j.dadd@improvision.com";
/* suggestion */
$subject = "Good ideas submission";
/* message */
if ($_POSTї"Name"] == "")
{
$message = $_POSTї"Suggestion"];
}
else
{
$message = $_POSTї"name"] submitted the following good idea $_POSTї"Suggestion"];
}
/* and now mail it */
mail($to, $subject, $message);
}
?>You are getting an "Undefined index" because you are trying to access index "Name" in the $_POST array, when one hasn't been entered.hairyjim wrote:Code: Select all
/* message */ if ($_POSTї"Name"] == "") { $message = $_POSTї"Suggestion"]; }
I think you need
Code: Select all
<?php
if (!isset($_POSTї"Name"]))
{
$message = $_POSTї"Suggestion"];
}
else
...
?>- twigletmac
- Her Royal Site Adminness
- Posts: 5371
- Joined: Tue Apr 23, 2002 2:21 am
- Location: Essex, UK
You may want to use the empty() function instead of the isset() function in order to also check that $_POST['Name'] does not contain an empty string.
this is equivalent to doing something like
Mac
Code: Select all
if (empty($_POSTї'Name']) {Code: Select all
if (!isset($_POSTї'Name']) || ($_POSTї'Name'] == '' || $_POSTї'Name'] == 0)) {Hey guys,
When I submit my form and say I submitted the following text:
Wouldn't it be great if we had a coke machine
Why does PHP put a \ before the ' like thus Wouldn''t
I thought it was me typing rubbish at first, but I can repeat this every time. I presume it is some sort of encoding, could you point me in the right directions?
James
When I submit my form and say I submitted the following text:
Wouldn't it be great if we had a coke machine
Why does PHP put a \ before the ' like thus Wouldn''t
I thought it was me typing rubbish at first, but I can repeat this every time. I presume it is some sort of encoding, could you point me in the right directions?
James
- twigletmac
- Her Royal Site Adminness
- Posts: 5371
- Joined: Tue Apr 23, 2002 2:21 am
- Location: Essex, UK
It adds slashes in case you want to put the data into a database. You can either turn off magic_quotes_gpc in your php.ini or if you don't have access to that or you want to make sure that your script will work as expected whether magic_quotes are on or off, you can use stripslashes() on each of the variables passed from the form to remove all the ''s.
Mac
Mac
PHP has special characters, like double quote to indicate a string, or an exclamation mark means logical NOT.
If you want to use these characters as they are, they need to be "escaped" so that PHP doesn't treat them as a special character, but as they are written.
Look at the colour formatting below to see how this affects a string:
Between the single quotes is a string, and PHP doesn't know that it is part of one long string. Compare with this.
If you want to use these characters as they are, they need to be "escaped" so that PHP doesn't treat them as a special character, but as they are written.
Look at the colour formatting below to see how this affects a string:
Code: Select all
<?php
print('Wouldn't it be great if we had a coke machine?');
?>Code: Select all
<?php
print('Wouldn''t it be great if we had a coke machine?');
?>Ahhh....
I could see that being a bit of a bummer if you have alot of form inputs
I have now changed the way in which my form works now, turned all the form inputs into variables at the top and stripped them there instead of every point I would specify the $_post in my code.
Cheers.
Works a treat.
James
I could see that being a bit of a bummer if you have alot of form inputs
I have now changed the way in which my form works now, turned all the form inputs into variables at the top and stripped them there instead of every point I would specify the $_post in my code.
Cheers.
Works a treat.
James
- twigletmac
- Her Royal Site Adminness
- Posts: 5371
- Joined: Tue Apr 23, 2002 2:21 am
- Location: Essex, UK
A quick way of stripping all your form inputs and to assign them to the same value as the particular form element's name attribute is to do:
If you just want to strip all of the $_POST array elements and store the new stripped value in $_POST you can do:
Mac
Code: Select all
foreach ($_POST as $key => $value) {
/* The double dollar sign on $key that follows is intended */
$$key = stripslashes($value);
}Code: Select all
foreach ($_POST as $key => $value) {
$_POSTї$key] = stripslashes($value);
}