Help required with a ForEach loop

[excelmaster]

Hello all,

 

I have the following code (which obviously has unnecessary redundancy) and I would like to replace it with a ForEach loop in order to make the code shorter, and perhaps more efficient.

        // validate input
        $valid = true;
        if (empty($store_name)) {
            $storeNameError = 'Please enter a Store Name';
            $valid = false;
        }

        if (empty($item_description)) {
            $itemDdescriptionError = 'Please enter an Item Description';
            $valid = false;
        }

        if (empty($qty_pkg)) {
            $qtyPkgError = 'Please enter Quantity or Package';
            $valid = false;
        }

        if (empty($pkg_of)) {
            $pkgOfError = 'Please enter the Count/Quantity';
            $valid = false;
        }

At this point (being a newbie) I'm trying to wrap my head around the ForEach statement, but all I can achieve is to tie-my-knickers-in-a-knot. I know this is childs-play for some of you here, and I hope you can help me get a working ForEach statement.

 

The following is the declaration of some related arrays (which potentially may be incorrect/inefficient as well):

    // Create an array of the fields we want to capture, along with their labels
    $fields = array(
        'store_name'=>'Store Name',
        'item_description'=>'Item Description',
        'qty_pkg'=>'Qty / Pkg',
        'pkg_of'=>'Pkg. Of',
        'price'=>'Price',
        'flyer_page'=>'Flyer Page #',
        'limited_time_sale'=>'Limited Time Sale Days',
        'nos_to_purchase'=>'Nos. to Purchase'
    );

    // Create an array-type variable to keep track of validation errors
    $fieldsErrorMsgs = array(
        'storeNameError'=>'Please enter a Store Name',
        'itemDescriptionError'=>'Please enter an Item Description',
        'qtyPkgError'=>'Please enter Qty or Pkg',
        'pkgOfError'=>'Please enter the Qty',
        'priceError'=>'Please enter the Price',
        'flyerPageError'=>'Please enter the Flyer Page #',
        'limitedTimeSaleError'=>'Please enter the Limited Time Sale Days',
        'nosToPurchaseError'=>'Please enter the No(s). to Purchase'
    );

And here's what I've tried...but there seems to be the following two problems with the ForEach portion of the code:

1) No errors are being displayed (inspite of the fact that I'm leaving all fields blank and clicking on the [Create] button

2) A row is being created (with all blank columns)

        $valid = true;
        $errors = array();
        
        foreach($fields AS $field=>$label){
            //The following line is using the ternary operator, it's basically a shorthand if/else assignment.  
            $errors[$field] = (!isset($values['store_name']) || !strlen($values['store_name'])) ? $errors[$label] = 'Mandatory field, input required!' : NULL;

            if(!strlen($errors[$label])){
                $valid = false;            
            }
        }  
 
        // insert data
        if ($valid) {
            $pdo = Database::connect();
            $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            $sql = "INSERT INTO shoplist (store_name,item_description,qty_pkg,pkg_of,price,flyer_page,limited_time_sale,nos_to_purchase,shopper1_buy_flag,shopper2_buy_flag,purchased_flag,purchase_later_flag)
                    VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
            $q = $pdo->prepare($sql);
            $q->execute(array($store_name,$item_description,$qty_pkg,$pkg_of,$price,$flyer_page,$limited_time_sale,$nos_to_purchase,$initialize_flag,$initialize_flag,$initialize_flag,$initialize_flag));
            Database::disconnect();
            header("Location: index.php");
        }

The following is the FORM portion of the code, which is where the logic for displaying the "error messages" is defiend

            <form class="form-horizontal" action="create.php" method="post">            
                <?php
                    foreach($fields AS $field=>$label){
                        //Print the form element for the field.
                        echo "<label>{$label}:<br>";
                        //If the field had an error, display it.
                        if(isset($errors[$field])){
                            echo ' <span class="error">'.$errors[$field].'</span><br>';
                        }
                        //Echo the actual input. If the form is being displayed with errors, we'll have a value to fill in from the user's previous submission.
                        echo '<input type="text" name="'.$field.'"';
                        if(isset($values[$field])){
                            echo ' value="'.$values[$field].'"';
                        }
                        echo '/></label>';
                    }
                ?>
                <div class="form-actions">
                    <button type="submit" class="btn btn-success">Create</button>
                    <a class="btn" href="index.php">Back</a>
                </div>
            </form> 

Appreciate the help guys.

 

Thanks

[ginerjm]

What is the $values array?  Perhaps you meant to use $_POST as your array name, not $values?

[Psycho]

You could create a foreach() loop, but I wound't. You may only be validating that each field is not empty, but you should have additional validations which won't be the same for each field. For example, I would be validation that the quantity type fields contain a numeric input. By creating a foreach loop you would have to add more logic into the loop to perform some validations for some fields and not for others. That just makes it more complicated. However, it could make sense to create functions for the different types of validations that you could call, as needed, for each applicable field.

[Ch0cu3r]

This is bit of the foreach loop does not seem right to me.

foreach($fields AS $field=>$label){
            //The following line is using the ternary operator, it's basically a shorthand if/else assignment.  
            $errors[$field] = (!isset($values['store_name']) || !strlen($values['store_name'])) ? $errors[$label] = 'Mandatory field, input required!' : NULL;
... 

You are validating the the store_name field for every field in the in the $fields array?

 

I think what you mean to do is loop through all the fields in $field, using the key for the index to  $_POST and then setting the corresponding error message in the $fieldsErrorMsgs array when the field does not validate. 

 

First we need come up with better approach which the $fields and $fieldsErrorMesg arrays. Which would be to merge them together so that the $fields array is constructed like so

$fields = array(
    'store_name' => array(
                       'label' => 'Store Name',
                       'error' => 'Please enter a store name'
                    ),
    ...
);

The foreach loop will then constructed like this

foreach($fields AS $field => $attr){

    if(!isset($_POST[ $field ]) || empty($_POST[ $field ]))
        $errors[] = $field; // add current field to the $errros array, this will be used later on to display the error message for the field
}

To prevent creating a new record when a field does not validated we'll check to see if the $errors array is empty

if(empty($errors))
{
    // add record to database
}

The loop for displaying the form fields should now be like

foreach($fields AS $field => $attr){
    //Print the form element for the field.
    echo "<label>{$attr['label']}:<br>";
    //If the field had an error, display it.
    if(in_array($field, $errors)) {
        echo ' <span class="error">'.$attr['error'].'</span><br>';
    }
    //Echo the actual input. If the form is being displayed with errors, we'll have a value to fill in from the user's previous submission.
    echo '<input type="text" name="'.$field.'"' . (isset($_POST[$field]) ? ' value="'.$_POST[$field].'"' : '') . ' /></label>';
}

[excelmaster]

@ginerjm: I figured I would have left out something...so thanks for checking and requesting.

 

Here's what you requested:

 

 

        // Create an array to hold the values we want to insert.
        $values = array();
        // For each of the fields we want, check if the field was POST(ed), and if so trim it and use it. Otherwise use NULL.
        foreach($fields AS $field=>$label){
            //The following line is using the ternary operator, it's basically a shorthand if/else assignment.  
            $values[$field] = isset($_POST[$field]) ? trim($_POST[$field]) : NULL;
        }        

 

@Psycho: Yes, you are correct in stating that I'm only validating for "not empty", and not for the other anomalies...and the only reason I'm doing this is because I'm taking baby-steps at this stage...trying to cross "one bridge at a time". Functions....to do more complex things....is certainly way down the road...but it would certainly help if you had the code handy and could pass it on to me.

 

Thanks

[ginerjm]

Ok - so now you are pulling in the input from post and stroring it in values for later use.  But at this same time you need to be doing the validations and grabbing the correct error messages and storing them somewhere to be used in your html.  And as others said, you need to be doing the specific type of validation that each field requires. 

 

I also noticed in your original post (of this new method) that you are just using simple vars in the array of values which I don't see being defined anywhere.  Forgot something else?  And remember - you have to have a value for every ? you placed in your prepared query statement and currently your logic doesn't handle that, unless you add code to ensure that every field is input.

Edited May 27, 2014 by ginerjm

[ginerjm]

My last post on this matter.

 

You are spending a lot of time trying to create something and having trouble because it is all so new to you.  There is nothing like the feeling of getting that first script working to your satisfaction.  This new method you are trying would be a wonderful thing to accomplish I'm sure, but you are showing a lot of confusion in trying to build it at this time.

 

One of the most important and most tedious parts of developing is pulling the data together and ensuring that it is what you expect, what will work, and isn't going to harm you.  Repetitive code chunks all doing similar things to like fields are a fact of life UNTIL you get to the point that you understand all those nuances of 'editing' and can at that time build yourself a working machine to help you accomplish this now and for the future.

 

My point is - why not take the time to write the code needed to complete this script, learning all the things that need to be done for each field and learn from it?  Then take that knowledge and use it again on your next script and gain more knowledge and when you get to the point that you feel comfortable building a sensible, efficient and safe tool, do it. 

 

Beginning coders always look back at their work from their early projects and wonder WTH they were doing.  Don't belabor this project with attempts at code that you may regret investing so much time in.

[excelmaster]

@Ch0cu3r: You are bang-on....in helping me reconstruct the $fields() array. That's exactly what I was thinking, but I didn't have the knowledge to apply it the way you did.

 

As a matter of fact, it was only last night that I reached the part where "multi-dimensional arrays - arrays within arrays" are explained in the book that I'm currently reading titled "PHP for Absolute Beginners"...and it sure is confusing for me right now.

 

So...with the code you provided, would I still need to use the $values() array...or have you done away with it?

 

@ginerjm:

 

>> I also noticed in your original post (of this new method) that you are just using simple vars in the array of values which I don't see being defined anywhere.  Forgot something else?

 

I'm not sure which portion of the code you're refering to...would you mind pointing it out, please?

 

>> And remember - you have to have a value for every ? you placed in your prepared query statement and currently your logic doesn't handle that, unless you add code to ensure that every field is input.

 

Woah! Really? I certainly didn't know that...much thanks for pointing it out to me....but now that you've mentioned it, it sure does make a lot of sense. I was initially validating all fields as mandatory input (without so much as knowing that they were required for the prepared statement), but then I changed it to allow some fields to be left blank.

 

Finally, I certainly appreciate the advice you've given me (in your last post on this matter).

 

Thanks.

[ginerjm]

In your lines after the prepare call you build an array with a set of php vars.  Where are they provided with values?

 

And what I meant about providing a value - you are grabbing $values items from your foreach loop but how do those input values get validated and then placed into the simple vars you are using in the array of inputs for the query?  Your foreach only creates a $values element for the inputs that are found in the POST array.  That means that $values will not have anything to validate nor to populate the corresponding var in the array of inputs.

 

Getting deeper and deeper.....

[excelmaster]

@ginerjm: Yes, perhaps I did miss posting some more of the code that's required for you to fully understand my problem. I guess that's the problem when I try to pull-out and post just bits-and-pieces of my code.

 

Perhaps it might help if I post the entire CREATE.PHP, since it's not extremely lengthy to begin with.

 

Thanks for bearing with me, and helping me learn along the way. BTW, in its long/inefficient form, my code was working just fine (except for the error messages part of course), and I wanted to take it to the next level, by making it shorter, and more efficient, and therefore this post

 

 

<?php
    error_reporting(E_ALL | E_STRICT | E_NOTICE);
    ini_set('display_errors', '1');                    
     
    require 'database.php';

    // Create an array of the fields we want to capture, along with their labels
    $fields = array(
        'store_name'=>'Store Name',
        'item_description'=>'Item Description',
        'qty_pkg'=>'Qty / Pkg',
        'pkg_of'=>'Pkg. Of',
        'price'=>'Price',
        'flyer_page'=>'Flyer Page #',
        'limited_time_sale'=>'Limited Time Sale Days',
        'nos_to_purchase'=>'Nos. to Purchase'
    );

    // Create an array-type variable to keep track of validation errors
    $fieldsErrorMsgs = array(
        'storeNameError'=>'Please enter a Store Name',
        'itemDescriptionError'=>'Please enter an Item Description',
        'qtyPkgError'=>'Please enter Qty or Pkg',
        'pkgOfError'=>'Please enter the Qty',
        'priceError'=>'Please enter the Price',
        'flyerPageError'=>'Please enter the Flyer Page #',
        'limitedTimeSaleError'=>'Please enter the Limited Time Sale Days',
        'nosToPurchaseError'=>'Please enter the No(s). to Purchase'
    );
    
    // Setup an "initialize variable" to (initially) set the four flag columns to "N", when creating a new record
    $initialize_flag = "N";
    
    if ( !empty($_POST)) {
        // Initialize all of the "error message" variables
        foreach($fieldsErrorMsgs AS $errorVariable=>$errorMessage){        
            $errorVariable = null;
        }
        
        // keep track of $_POST(ed) values
        $store_name = $_POST['store_name'];
        $item_description = $_POST['item_description'];
        $qty_pkg = $_POST['qty_pkg'];
        $pkg_of = $_POST['pkg_of'];
        $price = $_POST['price'];
        $flyer_page = $_POST['flyer_page'];
        $limited_time_sale = $_POST['limited_time_sale'];
        $nos_to_purchase = $_POST['nos_to_purchase'];        

        // Create an array to hold the values we want to insert.
        $values = array();
        // For each of the fields we want, check if the field was POST(ed), and if so trim it and use it. Otherwise use NULL.
        foreach($fields AS $field=>$label){
            //The following line is using the ternary operator, it's basically a shorthand if/else assignment.  
            $values[$field] = isset($_POST[$field]) ? trim($_POST[$field]) : NULL;
        }        
        
        $valid = true;
        $errors = array();
        
        foreach($fields AS $field=>$label){
            //The following line is using the ternary operator, it's basically a shorthand if/else assignment.  
            $errors[$field] = (!isset($values['store_name']) || !strlen($values['store_name'])) ? $errors[$label] = 'Mandatory field, input required!' : NULL;

            if(!strlen($errors[$label])){
                $valid = false;            
            }
        }        
        
        // insert data
        if ($valid) {
            $pdo = Database::connect();
            $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            $sql = "INSERT INTO shoplist (store_name,item_description,qty_pkg,pkg_of,price,flyer_page,limited_time_sale,nos_to_purchase,shopper1_buy_flag,shopper2_buy_flag,purchased_flag,purchase_later_flag)
                    VALUES(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
            $q = $pdo->prepare($sql);
            $q->execute(array($store_name,$item_description,$qty_pkg,$pkg_of,$price,$flyer_page,$limited_time_sale,$nos_to_purchase,$initialize_flag,$initialize_flag,$initialize_flag,$initialize_flag));
            Database::disconnect();
            header("Location: index.php");
        }
    }
?>

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <link   href="css/bootstrap.min.css" rel="stylesheet">
    <script src="js/bootstrap.min.js"></script>
    <!-- <link rel="stylesheet" type="text/css" media="all" href="style.css" />     -->        
</head>
 
<body>
    <div class="container">
        <div class="span10 offset1">
            <div class="row">
                <h3>Create an Item Record</h3>
            </div>

            <form class="form-horizontal" action="create.php" method="post">            
                <?php
                    foreach($fields AS $field=>$label){
                        //Print the form element for the field.
                        echo "<label>{$label}:<br>";
                        //If the field had an error, display it.
                        if(isset($errors[$field])){
                            echo ' <span class="error">'.$errors[$field].'</span><br>';
                        }
                        //Echo the actual input. If the form is being displayed with errors, we'll have a value to fill in from the user's previous submission.
                        echo '<input type="text" name="'.$field.'"';
                        if(isset($values[$field])){
                            echo ' value="'.$values[$field].'"';
                        }
                        echo '/></label>';
                    }
                ?>
                <div class="form-actions">
                    <button type="submit" class="btn btn-success">Create</button>
                    <a class="btn" href="index.php">Back</a>
                </div>
            </form>
        </div>
    </div> <!-- /container -->
  </body>
</html>
 

[Ch0cu3r]

 

 

So...with the code you provided, would I still need to use the $values() array...or have you done away with it?

If you look at my code I replaced where you used $values with $_POST. There is no need to have the input duplicated in two different variables.

[excelmaster]

@Ch0cu3r: Gotcha! It makes sense...to not use up more memory than is required.

 

One question: does the code below look right to you?

 

I ask because (on the input form) I am getting an error against each field that reads as follows:

 

Notice: Undefined variable: errors in /var/www/create.php on line 137

Warning: in_array() expects parameter 2 to be array, null given in /var/www/create.php on line 137

 

Line 137 contains:                        

if(in_array($field, $errors)) {

 

 

 

        $errors = array();
        
        foreach($fields AS $field => $attr){
            if(!isset($_POST[ $field ]) || empty($_POST[ $field ]))
                {
                    $errors[] = $field; // add current field to the $errros array, this will be used later on to display the error message for the field
                }
        }

 

Thanks

[Ch0cu3r]

Change line 137 to

if(isset($errors) && in_array($field, $errors)) {

[excelmaster]

@Ch0cu3r: That fixed that...but a bunch of errors popped up (all pertaining to the column names) when I clicked the [Create] button

 

The errors are:

 

Notice: Undefined variable: store_name in /var/www/create.php on line 108

Notice: Undefined variable: item_description in /var/www/create.php on line 108

Notice: Undefined variable: qty_pkg in /var/www/create.php on line 108

Notice: Undefined variable: pkg_of in /var/www/create.php on line 108

Notice: Undefined variable: price in /var/www/create.php on line 108

Notice: Undefined variable: flyer_page in /var/www/create.php on line 108

Notice: Undefined variable: limited_time_sale in /var/www/create.php on line 108

Notice: Undefined variable: nos_to_purchase in /var/www/create.php on line 108

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'nos_to_purchase' cannot be null' in /var/www/create.php:108 Stack trace: #0 /var/www/create.php(108): PDOStatement->execute(Array) #1 {main} thrown in /var/www/create.php on line 108

 

This is line 108:

 

 

$q->execute(array($store_name,$item_description,$qty_pkg,$pkg_of,$price,$flyer_page,$limited_time_sale,$nos_to_purchase,$initialize_flag,$initialize_flag,$initialize_flag,$initialize_flag));

[Ch0cu3r]

You are getting those errors because those variables are not defined. 

 

You need to define those variables before using them as part of your query.

[excelmaster]

@Ch0cu3r: Thank you (many times over) for your help, patience and understanding. Much appreciated!!!

 

You are indeed one-of-a-kind, and a genius as well.

 

Cheers