Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 5 of 5
  1. #1
    Regular Coder LearningCoder's Avatar
    Join Date
    Jan 2011
    Location
    The Pleiades
    Posts
    924
    Thanks
    76
    Thanked 29 Times in 29 Posts

    Post Can you review my code please?

    Hello.

    I think I have completed the validation of my code and was wondering if anyone can kindly give my any criticism as to anything I have missed or anything I can do better, which I'm sure both will generate some posts.

    There are little things I want to tweak but I wanted to know what some of the pros think (please go easy, im rubbish).

    Here is my contact.template.htm:
    PHP Code:
    <p id="contact_intro">It is a long established fact that a reader will be distracted by the readable content of a page when looking at its 
                          layout. The point of using Lorem Ipsum is that it has a more-or-less normal distribution of letters. It is a long 
                          established fact that a reader will be distracted by the readable content of a page when looking at its layout. 
                          The point of using Lorem Ipsum is that it has a more-or-less normal distribution of letters</p>
                          
    <form method="post" action="index.php?page=contact">
         <fieldset>
             <legend>Gardenable Contact Form</legend>
             
                 <p class="form_heading">Your Details</p>
                 <p class="form_instructions">Please leave us your details so we can contact you back!</p>
                 <hr class="form_hr" />
                 <p><label for="name">Name:</label><input type="text" name="name" id="name" size="36" maxlength="36" /><span class="red">*</span></p> 
                 <p><label for="email">Email:</label><input type="text" name="email" id="email" size="36" maxlength="70" /></p> 
                 <p><label for="phone">Phone:</label><input type="text" name="phone" id="phone" size="36" maxlength="16" /><span class="red">*</span></p>
                 <p><label for="user_comments">Additional Comments:</label><textarea name="user_comments" id="user_comments" rows="5" cols="34" maxlength="400"></textarea></p>
                 
                 <hr />
             
                 <p class="form_heading">Product Details</p>
                 <p class="form_instructions">If you wish to <span class="italic">order</span> or <span class="italic">query</span> a product, please specify below.</p>
                 <hr class="form_hr" />
                 
                 <p><label for="product">Product:</label>
                     <select name="product_options">
                     <option value="default">Choose a product...</option>
                         <option value="benches">Benches</option>
                         <option value="bin_stores">Bin Stores</option>
                         <option value="bird_housing">Bird Housing</option>
                         <option value="gates">Gates</option>
                         <option value="pet_housing">Pet Housing</option>
                         <option value="planters">Planters</option>
                         <option value="sheds">Sheds</option>
                         <option value="tables">Tables</option>
                     </select>
                 </p>
                 <p><label for="product_ref">Product ID:</label><input type="text" name="product_ref" id="product_ref" size="20" maxlength="7" />
                 <p><label for="product_comments">Product Comments:</label><textarea name="product_comments" id="product_comments" rows="5" cols="34" maxlength="400"></textarea></p>
                 
                 <p><input type="submit" name="submit" value="Submit" />
                    <input type="reset" name="reset" value="Reset" />
                 </p>
                 <span id="form_required">Fields marked with a red asterix (<span class="red">*</span>) are required.</span>
         </fieldset>
    </form>

    <div id="error_div">
         <?php if(isset($output)){ print_r($output);} ?>
    </div>
    Here is validation relating to it:
    PHP Code:
    if(isset($_POST['submit'])){
             
         
    //if script gets here, the user submitted the form. delete last element (submit button) as we do not need it.
         
    array_pop($_POST);
         
         
    //create array to hold any errors.
         
    $errors = array();
        
         
    //firstly, check to see if my required fields contain any data. if they dont we add errors to the error array.
         
    if(empty($_POST['name']) || empty($_POST['phone'])){
             
    $errors[] = "You must fill in the required fields marked with a RED asterix(*).";
         }

         
    //check to see if the errors array contains anything. if it does, we need to send the user back to the form and display the error.
         //do not carry on if the if statement executes because we dont want to process any more as we know we are going to have to send them back anyway.
         
    if(!empty($errors)){
             
    $output $errors;
         }
         else{
         
    //if the code reaches here, we have data inside the two required fields so carry on processing all of the data now.
         //pass a reference of the value so that if any ARE set to string NULL, it also changes the original $_POST value.
         
    foreach ($_POST as $post => &$value) {
             if(
    $value == ""){
                 
    $value "NULL";
             }
             else{
                 switch (
    $post) {
                     
                     case 
    "name"
                         if(!
    ctype_alpha($value)){
                             
    $errors[] = "The name field can only contain alphabetical characters.";//specify just a first name in form
                         
    }
                     break;
                     
                     case 
    "email"
                         if(!
    filter_var($value,FILTER_VALIDATE_EMAIL)){
                             
    $errors[] = "You did not enter a valid email address.";//give an example of an email someone@provider.com in form
                         
    }
                     break;
                     
                     case 
    "phone":
                         
    //replaces all characters that are NOT digits 0-9.
                         
    $value preg_replace("/\D/","",$value);
                         
                         
    //we need to check if it is not equal to an empty string again because if they entered all letters, the preg_replace will replace them
                         //and my second if statement here will show an undefined index error. if it is an empty string, add to error array and break out of case
                         //prematurely.
                         
    if($value == ""){ $errors[] = "You did not enter a phone number."; break;}
                         
                         
    //checks to see if the first character of the string is not equal to a 0 or if the length of the string isn't 11 (which means its not valid).
                         
    if($value[0] != "0" || strlen($value) != 11){
                              
    $errors[] = "You did not enter a valid phone number.";
                         }
                     break;
                     
                     case 
    "user_comments":
                         
    $len strlen($value);
                         
                         if (
    $len 400){
                              
    $less = ($len 400);
                              
    $errors[] = "You must enter {$less} LESS characters in the 'Additional Comments' field.";
                         }
                     break;
                     
                     case 
    "product_options":
                         
    //not sure how to validate the <select> options. dont think I need to as it will always be 'default' or a product name.
                         
                     
    break;
                     
                     case 
    "product_ref":
                         
                         
    //checks to see if the length of the string is not equal to 7
                         
    if(strlen($value) != 7) {
                            
    $errors[] = "The product id you entered was not long enough, must be 7 numbers.";
                             
                         }
                         
    //checks to see if any of the characters entered were not digits. if this executes, we know that the user entered something different
                         //than 7 digits so there is no need to carry on and check the ref no against the records so we break out of case prematurely.
                         
    if(!ctype_digit($value)){
                            
    $errors[] = "Product id's can only contain numbers.";
                            break;
                         }
                         
                         
    //prepared statement which checks the product ref no submitted against a product ref in the database. 
                         
    require("core/prepared_select_pref.php");
                         
                         if(
    $row != 1){
                             
    $errors[] = "Your Product ID did not match one of our products.";
                         }
                         
                     break;
                     
                     case 
    "product_comments":
                         
    $len strlen($value);
                         
                         if(
    $len 400){
                             
    $less = ($len 400);
                             
    $errors[] = "You must enter {$less} LESS characters in the 'Product Comments' field.";
                         }
                     break;
                 }
             }
             
         }
         }
         
         
    //if the error array contains data, we had some errors during validation, so we display all of these error(s) to the user.
         
    if (!empty($errors)){
            
            
    $output "<ul>";
                foreach (
    $errors as $err => $error_value){
                    
    $output .= "<li>".$error_value."</li>";
                    
    $output .= "<hr>";
                }
            
    $output .= "</ul>";
         }
         else{
    //if there were no errors after all the validation, insert data to database.
            
    require("core/prepared_insert.php");
            if(
    $row >= 1){
                
    $output "Your information has successfully sent!";
            }
            else{
                
    //maybe send their information to my email instead if there is an issue with insert....probably the best idea rather than displaying an error.
                
    $output "There was an error receiving your information.";
            }
         }
         

    Thanks very much for any help/tips you can give me.

    Kind regards,

    LC..
    Last edited by LearningCoder; 11-16-2012 at 02:01 AM.

  • #2
    Regular Coder LearningCoder's Avatar
    Join Date
    Jan 2011
    Location
    The Pleiades
    Posts
    924
    Thanks
    76
    Thanked 29 Times in 29 Posts
    Really, no one willing to give it a brief scan to see if I've missed anything?

    I've never actually built a robust script, just want opinions. I've tried entering crap into the form every single possible way and my errors seem to deal with them all.

    Regards,

    LC.

  • #3
    Senior Coder
    Join Date
    Feb 2011
    Location
    Your Monitor
    Posts
    4,398
    Thanks
    61
    Thanked 535 Times in 522 Posts
    I've quickly scanned it and stopped on the very first line.

    I'll let you work it out from my sig - I've replied to your threads more than enough times for you to have seen it
    See my new CodingForums Blog: http://www.codingforums.com/blogs/tangoforce/

    Many useful explanations and tips including: Cannot modify headers - already sent, The IE if (isset($_POST['submit'])) bug explained, unexpected T_CONSTANT_ENCAPSED_STRING, debugging tips and much more!

  • #4
    Regular Coder LearningCoder's Avatar
    Join Date
    Jan 2011
    Location
    The Pleiades
    Posts
    924
    Thanks
    76
    Thanked 29 Times in 29 Posts
    Aha been a time!

    You have indeed! I'll give your sig a read.

    Kind regards,

    LC.

  • #5
    Regular Coder LearningCoder's Avatar
    Join Date
    Jan 2011
    Location
    The Pleiades
    Posts
    924
    Thanks
    76
    Thanked 29 Times in 29 Posts
    Ok just read your topic in your signature. Notice you check for other form fields and not the submit button. That seems straight forward enough for me. I also noticed Fou stated we should check for every single field (which is what I have done in my other scripts for that demo site when I first started it). Think I might go with your solution of checking another field is set for example my $_POST['name'] field.

    I'm thinking of slightly modifying my code to this:

    PHP Code:
    if (isset($_POST['name'])){
        
    //start my processing
        
    (isset($_POST['submit'])) ? array_pop($_POST['submit']) : ""



    If you relate that to the code above to my original code post, do you think this will now cover all eventualities?

    Edit: Just changed my code to what I wrote above and it works great.

    I'm actually on XP so I can't even get IE9, which in my opinion is an absolute joke. I have an MS system which can't run the default MS browsers latest version, what kinda crap are they pulling? Anyway the highest version I have is 8. I'll play around with the submit thing and see the result I get to gain a better understanding.

    Plus, would you be willing to just briefly scan my code and see what you think? It seems a little basic to me and I feel there is more I can do to secure it. Like I've said I've tried entering crap into the fields every way possible and my processing seems to catch every possible error. So I know that I'm on the way to having a good script, I just want some opinions on how to improve it.

    It works as I want so it's more of a 'you could do this' or change that foreach loop etc type of answer im looking for. Had over 100 views with 1 reply....bad times. Is reviewing completed code something that isn't done here I think I've seen on other forums a section for 'code critique'?

    Thanks for your reply!

    Regards,

    LC.
    Last edited by LearningCoder; 11-17-2012 at 07:33 AM.


  •  

    Posting Permissions

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