5
\$\begingroup\$

I need help with this code. How can I make it shorter? How can I create an array with the names, for example, errors, and modify code without so many If statements? I still can't solve it. I'm a beginner in PHP.

$error1 = $error2 = $error3 = $error4 = $error5 = $error6 = $error7 = $error8 = ""; 

if ($_SERVER["REQUEST_METHOD"] == "POST") {
if (empty($_POST["fname"])) {
  $error1 = "fill in fname";
}  
if (empty($_POST["lname"])) {
  $error2 = "fill in lname";
} 
if (empty($_POST["street"])) {
  $error3 = "fill in street";
}  
if (empty($_POST["city"])) {
  $error4 = "fill in city";
} 
if (empty($_POST["postcode"])) {
  $error5 = "fill in postcode";
} 
if (empty($_POST["country"])) {
  $error6 = "fill in country";
} 
if (empty($_POST["email"])) {
  $error7 = "fill in email";
} 
if (empty($_POST["phone"])) {
  $error8 = "fill in phone";
} 
if (empty($_POST["fname"]) ||
    empty($_POST["lname"]) ||
    empty($_POST["street"]) ||
    empty($_POST["city"]) ||
    empty($_POST["postcode"]) ||
    empty($_POST["country"]) ||
    empty($_POST["email"]) ||
    empty($_POST["phone"])) {
} else {
  $form_items = $_POST;
    $document = new DOMDocument("1.0", "utf-8"); 
    if (is_file('form_data.xml')) 
    {
        $document->load("form_data.xml"); 
        $xmlko = $document->getElementsByTagName('xml')->item(0); 
    }
    else 
    {
        $xmlko = $document->createElement("xml"); 
        $document->appendChild($xmlko); 
        $xmlko = $document->getElementsByTagName('xml')->item(0); 
    }
            $form = $document->createElement("form"); 
                $new_user = $document->createElement("user"); 
                foreach ($form_items as $ite => $val) { 
                    $this_user = $document->createElement($ite, $val); 
                    $new_user->appendChild($this_user); 
                }
               $form->appendChild($new_user); 
           $xmlko->appendChild($form); 
    $document->save("form_data.xml"); 

  }
}

?>

Here is HTML code. Everything is in one PHP file. 

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <title>form</title>
    <link rel="stylesheet" href="CSS/styly.css">
  </head>
  <body>
    <h1>FORM</h1>
    <form action="" method="post">
      <p>FName:</p>
      <input type="text" name="fname"/><span class="error"><?php echo $error1?></span>
      <p>LName:</p>
      <input type="text" name="lname"/><span class="error"><?php echo $error2?></span>
      <p>Street:</p>
      <input type="text" name="street"/><span class="error"><?php echo $error3?></span>
      <p>City:</p>
      <input type="text" name="city"/><span class="error"><?php echo $error4?></span>
      <p>Postcode:</p>
      <input type="text" name="postcode"/><span class="error"><?php echo $error5?></span>
      <p>Country:</p>
      <input type="text" name="country"/><span class="error"><?php echo $error6?></span>
      <p>E-mail:</p>
      <input type="text" name="email"/><span class="error"><?php echo $error7?></span>
      <p>Phone:</p>
      <input type="text" name="phone"/><span class="error"><?php echo $error8?></span> 
      <input type="submit" value="SUBMIT"/> 
    </form>

   <?php 

      $document = new DOMDocument("1.0", "utf-8");
      if (is_file('form_data.xml')) {
        $document->load("form_data.xml");
        $users = $document->getElementsByTagName("user");

        echo "<table>";

            echo '<tr>';
            foreach($users->item(0)->childNodes as $data) {
                echo '<th>'.$data->nodeName."</th>";
            }
            echo '</tr>';


            foreach($users as $user){
                echo '<tr>';
                foreach($user->childNodes as $data) {
                    echo '<td>'.$data->nodeValue."</td>";
                }
                echo '</tr>';
            }

        echo "</table>";
      }

   ?>

  </body>
</html>

All data from the HTML form is inserted into the table below the form.

\$\endgroup\$
1
  • \$\begingroup\$ I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$
    – Sᴀᴍ Onᴇᴌᴀ ♦
    Commented Mar 5, 2019 at 7:18

2 Answers 2

Reset to default
6
\$\begingroup\$

Instead of individual variables, you should use an array.

$errors = [];
foreach (['fname', 'lname', 'street'] as $field) {
    if (empty($_POST[$field])) {
        $errors[$field] = "$field must be filled in";
    }
}
…
if (!empty($errors)) {
    …
}

And then, in the HTML:

<?php echo $errors['fname'] ?>
\$\endgroup\$
3
\$\begingroup\$

@Roland Illig has a good point: the field names can be put into an array. Perhaps it would be best to use one array for the labels of the fields and then use that array to generate the field names.

$fieldLabels = array("FName","LName","Street","City","Postcode","Country","E-mail","Phone");

To get the field name (e.g. when iterating over each label), a function can be applied to the label. For the case of the labels above, the name can simply be derived by removing the hyphen (i.e. -) and making the label lowercase. For those operations, str_replace() and strtolower() can be used. This logic could be abstracted into a function (or static method if a class was defined) and called when necessary: 

function GetFieldNameFromLabel($label) {
    return str_replace('-', '', strtolower($label));
}

Then when checking if all field have been completed, that function can be called. The labels can be iterated over, to check if there is a non-empty value in the POST data (i.e. $_POST) - if so, store such a value in an array (e.g. $validValues).

$validValues = array();
foreach($fieldLabels as $fieldLabel) {
    $fieldName = GetFieldNameFromLabel($fieldLabel);
    if (!empty($_POST[$fieldName])) {
        $validValues[$fieldName] = $_POST[$fieldName];
    }
}

Then the condition for adding the entry in the XML can simply be whether the number of values in that array of valid values is the same as the number of labels:

if (count($validValues) == count($fieldLabels)) {
    //store values from $validValues in XML
}

Then for generating the form, the fields can be iterated over, and the error message can be added conditionally. One could even conditionally add the error message container (i.e. <span class="error">). Some may argue this logic should be separated from the HTML - that could be achieved using a template system (e.g. Smarty, Symfony Twig, etc.) - that could also be used for the display of existing data.

<form action="" method="post">
    <?php
    foreach($fieldLabels as $fieldLabel) {
        $fieldName = GetFieldNameFromLabel($fieldLabel);
        $error = '';
        if ($_SERVER["REQUEST_METHOD"] == "POST" && !array_key_exists($fieldName, $validValues)) {
            $error = "Fill in ".$fieldName;
        }
        echo "<p>$fieldLabel:</p>";
        echo '<input type="text" name="'.$fieldName.'"/><span class="error">'.$error.'</span>';
    }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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