4
\$\begingroup\$

I'm looking for a review of my code for better design (to make sure it's more readable and legible).

<?php

// define variables and set to empty values
$nameErr = $phoneErr = $streetErr = $cityErr = $stateErr = $zipErr = "";
$name = $phone = $street = $city = $state = $zip = "";

// start script if the form is submitted
if ($_SERVER["REQUEST_METHOD"] == "POST") {

    if (empty($_POST["name"])) {
    $nameErr = "Name is required";
    }
    else {$name = test_input($_POST["name"]);
            // check if name only contains letters and whitespaces
        if (!preg_match("/^[a-zA-Z ]*$/",$name)) {
            $nameErr = "Only letters and white space allowed"; 
        }
}

    if (empty($_POST["phone"])) {
        $phoneErr = "Phone # is required";
    }
    else {$email = test_input($_POST["phone"]);
        // check if phone only contains numbers and dashes in correct format
        if (!preg_match("/^[0-9]{3}-[0-9]{3}-[0-9]{4}$/",$phone)) {
            $phoneErr = "Please resubmit Phone # in the following format: 555-     555-5555"; 
        }
    }

    if (empty($_POST["street"])) {
    $streetErr = "Address is required";
    }
    else {$street = test_input($_POST["street"]);
     // check if address syntax is valid
      if (!preg_match("/^[0-9a-zA-Z. ]+$/",$street)) {
       $streetErr = "Address appears to be invalid.";
      }
    }

    if (empty($_POST["city"])) {
      $cityErr = "City is required";
    }
    else {$city = test_input($_POST["city"]);
 // check if city syntax is valid
      if (!preg_match("/^[a-zA-Z ]*$/",$city)) {
      $cityErr = "City appears to be invalid." ; 
     }
    }

    if (empty($_POST["state"])) {
     $stateErr = "State is required";
} 
    else {$state = test_input($_POST["state"]);
 // check if state is valid uppercase abbv. and two letters
 if (!preg_match("/^(A[LKSZRAP]|C[AOT]|D[EC]|F[LM]|G[AU]|HI|I[ADL N]
     |K[SY]|LA|M[ADEHINOPST]|N[CDEHJMVY]|O[HKR]|P[ARW]|RI|S[CD] 
     |T[NX]|UT|V[AIT]|W[AIVY])$/",$state)) {
     $stateErr = "State appears to be invalid. (Hint...Use Upper-case!)" ; 
   }
}

if (empty($_POST["zip"])) {
    $zipErr = "Zip Code field is required";
} 
else {$zip = test_input($_POST["zip"]);
    // check if zip only contains numbers and dashes in correct format
    if (!preg_match("/^[0-9]{5}-[0-9]{4}$/",$phone)) {
        $phoneErr = "Please resubmit Zip Code in the following format: 55555-5555"; 
    }
}
}

function test_input($data) {
   $data = trim($data);
   $data = stripslashes($data);
   $data = htmlspecialchars($data);
   return $data;
}
?>
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Welcome. Could you please describe this code for better context? Does it work as intended? If it does work, what type(s) of review are you seeking? \$\endgroup\$ – Jamal May 22 '14 at 1:45
  • 1
    \$\begingroup\$ What does your test_input function does ? \$\endgroup\$ – matheussilvapb May 22 '14 at 1:49
  • \$\begingroup\$ The code works yes, this code validates various input HTML forms. I was looking for a markup of my code for better structure of the syntax. (to make it more readable and legible) \$\endgroup\$ – user42910 May 22 '14 at 1:53
  • \$\begingroup\$ Make sure your code is not vulnerable to PHP injection, you should use htmlspecialchars() \$\endgroup\$ – matheussilvapb May 22 '14 at 2:03
1
\$\begingroup\$
  1. you should be using arrays to store your variable.

  2. your coding if syntax are not indented and formatted well.
    https://stackoverflow.com/questions/253030/best-way-to-format-if-statement-with-multiple-conditions

  3. it is better to use

    if ($_POST) { }

    /** rather than **/

    if ($_SERVER["REQUEST_METHOD"] == "POST") { }

in your case.

if anything else read about PSR1 or PSR2 coding standard.

  1. http://www.php-fig.org/psr/psr-1/
  2. http://www.php-fig.org/psr/psr-2/
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.