Jump to content

For each Loop issue!!!


George Botley

Recommended Posts

I'm starting to get peeved with PHP today, after many hours of trying to sort an issue - I'm sure you all know what I mean.

 

Once again, the issue lies with foreach loops which I have never been able to fully understand, especially when dealing with multiple arrays.

 

The code is for a CMS, where a restaurant owner can update his menu's and choose whether to show them on the website using a tick box.

 

Here's a screenshot..

 

screenshot20110831at164.png

 

Essentially, the issue I am having is should I click say, the bottom tickbox and click Update it will for some reason, tick the second in the database.... In reverse, if all are ticked, and I untick the bottom, it will untick the box before it.... it is sooo frustrating..

 

From what I can see, it is likely to be where the array, where ticked boxes have a value, and non-ticked ones don't... but I tried to sort this in my foreach loop setting an empty value to off....

 

Here is the HTML code:

 

<form action="./includes/scripts/menus/update.php" enctype="multipart/form-data" method="post">

<button type="submit" onclick="return confirm('Updating Menu Changes...\n\nUpdating PDF files may time some time.. \n\nDo not navigate away from this page until you recieve a success or failure confirmation.\n\nClick Cancel to make changes.'); { return alert('noob'); }">Update Menu Changes</button>
<button type="button" onClick="document.location.href='./includes/scripts/menus/new_row.php'">Add New Menu Item...</button>

<br /><br />

<table border="0" width="962" cellpadding="0" cellspacing="0">

    <tr style="padding-bottom: 3px;">    
        <th style="text-align: center;" width="195.4">Active</th>
        <th style="text-align: center;" width="195.4">Menu</th>
        <th style="text-align: center;" width="195.4">Current Menu</th>
        <th style="text-align: center;" width="195.4">Change Menu (PDF format only)</th>
        <th style="text-align: text-align: center;" width="195.4">Tasks</th>
    </tr>
    
    <? 
    
    $query = "SELECT * FROM menus ORDER by id ASC";
    $query = mysql_query($query) or die(mysql_error());
    
    //Count rows
    $num_rows = mysql_num_rows($query);
    
    //If no rows exist
    if($num_rows=="0") { ?>
        
        <tr style="padding-bottom: 3px;">            
            <td colspan="4" style="text-align: center;">There are currently no menus listed in the Pen Mill Hotel database.</td>       
        </tr>
        
    <? }
    
    //If new rows exist
    elseif($num_rows!=="0") {
        
                $i = "0";
    
    while($row=mysql_fetch_array($query)) {
    
        //Database Variables
        $name = $row["menu"];
        $id = $row["id"];    
        $location = $row["location"];
        
        //Establish if a menu is present in database for current entry
        if(empty($location)) {
            $current = "<a style=\"cursor: pointer;\" onclick=\"alert('No menu found!');\" title=\"View Listed Menu\">View Current Menu</a>";
        }
        
        elseif(!empty($location)) { 
            $current = "../includes/menus/" . $row["location"];
            $current = "<a href=\"$current\" title=\"View Listed Menu\" />View Current Menu</a>";
        }
        
        //Establish whether to check tickbox or not.
        $active = $row["active"];
            if($active=="on") { $active = " checked=\"checked\" "; }
            elseif($active!=="on") { $active = " "; }
        
    ?>
    
    <tr style="padding-bottom: 3px;" id="<? echo "$id"; ?>">
    
        <td style="text-align: center;">
                <input type="checkbox"<? echo "$active"; ?>name="active[<? echo "$i"; ?>]" />  
        </td>
        
        <td style="text-align: center;">
            <input type="text" style="padding: 0px;" name="name[]" value="<? echo "$name"; ?>" />
        </td>
        
        <td style="text-align: center;">
            <? echo $current; ?>
        </td>
        
        <!-- Tell PHP, max file size is equal to 5MB (in bytes) -->
        <input type="hidden" name="MAX_FILE_SIZE" value="5242880" />
        
        <td style="text-align: center;"><input type="file" name="menu_file[]" /></td>
        
        <td style="text-align: center;">
            <a href="./includes/scripts/menus/delete.php?id=<? echo "$id"; ?>"
            onclick="return confirm('Are you sure you wish to delete this menu?\n ');">
                Delete
            </a>
         </td>

    </tr>
    
    <input type="hidden" name="id[]" value="<? echo $id; ?>" />
    
    <? $i++; ?>

<? } } ?>
    
</table>

<br />

<button type="submit" onclick="return confirm('Updating Menu Changes...\n\nUpdating PDF files may time some time.. \n\nDo not navigate away from this page until you recieve a success or failure confirmation.\n\nClick Cancel to make changes.');">Update Menu Changes</button>
<button type="button" onClick="document.location.href='./includes/scripts/menus/new_row.php'">Add New Menu Item...</button>

</form>

 

And here is the PHP, baring in mind it will also deal with a file upload, but that isn't the issue at the moment...

<?

/* 

This script is for use on the Manage Menu's pages and updates all menu items.

*/

/*==================================*/
/* DATE BASE CONFIGURATION            */
/*==================================*/

include_once "../../config/config.php";

/*==================================*/
/* SET VARIABLES                    */
/*==================================*/

$id = $_POST["id"];
$active = $_POST["active"];
$name = $_POST["name"];
$menu_file = $_FILES['menu_file'];
$i = "0";

/*==================================*/
/* Loop Each Menu & Update             */
/*==================================*/

foreach($id as $key) {
    
    //Set Sub Variables
    if(empty($active["$i"])) { $active["$i"] = "off"; }
    $active_post = $active["$i"];
    $name_loop = $name["$i"];
    
    //Set File Variables
    $file_name = $menu_file["name"]["$i"]; //name of file on users machine
    $file_type = $menu_file["type"]["$i"]; //type of file being uploaded
    $file_size = $menu_file["size"]["$i"]; //size of the uploaded file in bytes
    $file_error = $menu_file["error"]["$i"]; //returned PHP error codes for upload
    $file_temp = $menu_file["tmp_name"]["$i"]; //temporary name on server
    
    //Handle Uploaded File (if uploaded)
    if(!empty($file_name)) {
        
        //FILE ERROR CHECKS
        //If file type is not PDF.
        if($file_type!=="application/pdf") {
    
            $pass = "0";
            header("Location: ../../../index.php?page=Menus&error=Incorrect_File_Type");
            
        }
        
        //If file type is greater than 5MB.
        if($file_size>"5242880") {
    
            $pass = "0";
            header("Location: ../../../index.php?page=Menus&error=Incorrect_File_Size");
            
        }
        
        //If above error checks pass
        if($pass!="0") {
            
            //Get todays date for file rename
            $today = date("d-m-Y");
            $uploaddir = "/home/penmillh/en/includes/menus/";
            $upload_name = str_replace(" ", "", $name_loop) . $today . ".pdf";
            $uploadfile = $uploaddir . $upload_name;
            
            //Move file only if PHP upload status is correct
            if($file_error!="UPLOAD_ERR_OK") {
            
                header("Location: ../../../index.php?page=Menus&error=File_Upload_Error");
                
            }
            
            //If PHP status is okay, upload file and update table.
            if($file_error=="UPLOAD_ERR_OK") {
            
                //Upload
                move_uploaded_file($file_tmp, $uploadfile);
                
                //Update Database
                $query = "UPDATE menus SET active='$active', menu='$name_loop', location='$upload_name' WHERE id='$value'";
                $query = mysql_query($query) or die(mysql_error());
                
                header("Location: ../../../index.php?page=Menus&error=File_Upload_Success");
                
            }    
            
        }
        
    }
    
    //If no uploaded file, update database records
    if(empty($file_name)) {
             
        //Update Database        
        $query = "UPDATE menus SET active='$active_post' WHERE id='$key'";
        $query = mysql_query($query) or die(mysql_error());
        
//        header("Location: ../../../index.php?page=Menus&error=Menu_Change_Success");
             
    }
    
    $i++;

}


?>

 

Now, I have probably made this INCREDIBLY complicated, but that's all I could remember from my previous experience with foreach loops....

 

Any ideas?

 

Link to comment
Share on other sites

Not sure what your issue is with understanding foreach() loops, they simply iterate through each record in an array.

 

But, let me be blunt - your is very disorganized and just plain messy. I could spend a while going over all the issues I see (i.e. don't use PHP short tags). I'm not saying this to be mean. You need to step back and plan out what you are doing and then execute it. I can tell you are creating the logic as you write the code - and some of the logic is just plain poor. For example:

        $active = $row["active"];
            if($active=="on") { $active = " checked=\"checked\" "; }
            elseif($active!=="on") { $active = " "; }

Why do you use an elseif() and why are you using a string variable for a true/false value? You should be using a tinyint field with a 1/0. That logic should look something like this:

$active = "";
if($row["active"]=='on')
{
    $active = 'checked="checked" ';
}

 

Or just use the ternary operator:

    $active = ($row["active"]=='on') ? 'checked="checked" ' : '';

 

 

I've looked briefly over your code and am not going to try and "fix" it because you are taking the wrong approach. You should NEVER run queries in loops. There is always a better way. Anyway, this is the approach you should be taking:

 

For the form, create the checkboxes with a name as an array and the value as the ID of the record:

<input type="checkbox" name="active[]" value="<?php echo $id; ?>

 

Now, as you should know only "checked" checkbox inputs are passed in the POST data. So, you will not get a list of only checked records and not the unchecked records. You can easily update all the records with just TWO queries. Example:

 

$checkedIDsAry = $_POST['active'];
//Convert values to INTs for DB security
array_walk($checkedIDsAry, 'intval');
//Convert to comma separated string
$checkedIDsStr = implode(',', $checkedIDsAry);

//Create and run one query to update checked records
$query = "UPDATE table SET active = 1 WHERE id IN ($checkedIDsStr)";
mysql_query($query);

//Create and run one query to update unchecked records
$query = "UPDATE table SET active = 0 WHERE id NOT IN ($checkedIDsStr)";
mysql_query($query);

Link to comment
Share on other sites

Well, do I say thanks?

 

 

In some ways yes, thank you for your comment.. but I'm sure you once were scrappy with your code at some point. We all learn new things every day.

 

 

I came on here to ask for assistance in how to improve the loop, as, from a previous topic on here a while back, I had been advised to use foreach loops when there was more than one line of data to modify in a database from the same form fields in the same form.

 

 

If I was to re-write the script from the beginning then, what would you suggest (not asking for a complete script from yourself), but just wish to understand where I should begin...

 

 

Why is it not good practice to loop it? as it's not only the checkboxes I want to deal with, it's other fields too?

Link to comment
Share on other sites

Well, do I say thanks?

 

In some ways yes, thank you for your comment.. but I'm sure you once were scrappy with your code at some point. We all learn new things every day.

 

Yes, you *should* say thanks. You came here asking for assistance and I provided it, along with some constructive criticism. I'm sorry if the manner in which I provided that criticism damaged your fragile sensibilities. Yes, I have not always written code as I do now, but I always strived to write code that was logical and adhered to some sort of comprehensible format. Sorry, but unless someone started writing code yesterday I don't buy the "I'm new to programming" defense. You don't have to be a good programmer to write clean code. But, it is just criticism, do with it what you will. And, I could really give a rats ass if you thank me or not. I don't come here and invest my time and energy to get validation of my self worth. I do it because I enjoy helping people.

 

I came on here to ask for assistance in how to improve the loop, as, from a previous topic on here a while back, I had been advised to use foreach loops when there was more than one line of data to modify in a database from the same form fields in the same form.

 

...

 

Why is it not good practice to loop it? as it's not only the checkboxes I want to deal with, it's other fields too?

 

Running queries in loops is incredibly inefficient. It uses up a lot of CPU/Memory resources and as your application and/or user base grows will create performance issues. True, you probably won't have any significant issues with this particular implementation if you used a loop, but why build it the wrong way when 1) The right way is much simpler and 2) Doing it the wrong way instills bad practices that would then get implemented in functionality where is would be a problem.

 

f I was to re-write the script from the beginning then, what would you suggest (not asking for a complete script from yourself), but just wish to understand where I should begin...

 

If it were me, I would have one page where a user can edit the "Active" status for all records, but not allow editing of the Name or PDF (on that page). I would make the user click a link/button to go to a page to edit that information individually for each record (in addition to the active status if they wish). That's just a preference of mine, but there are technical/usability reasons why I would do that. For one, if you allow the user to submit all those changes for multiple records on one page you have to validate the input for all the records (which you don't appear to be doing and are completely open to SQL Injection!). Then if there is a validation error for one record it becomes more difficult to handle. Also, if the user is only needing to update one or a couple records you still have to process ALL the records, that's just inefficient. There are other reasons too, but I'm not going to write a book here.

 

Ok, so let's say you do want to have a page to update all the information for all the records in one swoop. That can be done - again without running queries in a loop. In addition to implementing validation and error handling which you are currently lacking, you would implement a process to gather all the data to be updated and create a single query using a MySQL CASE statement. You could use your current code as a base for this. But, you would still have to fix the problem you have.

 

In my last post I stated:

as you should know only "checked" checkbox inputs are passed in the POST data

 

But, I guess you don't know that because that is the cause of your problem. Currently your input fields are being created as arrays (good), but you are not defining an index for them (bad). You assumed that the first checkbox in the post data would belong to the first text field and the first file field. But, if you only check the 3rd and 4th checkboxes then the 1st and 2nd aren't included in the POST data, so the first checkbox value in the POST data will belong tho the 3rd record and the 2nd checkbox value will belong to the 4th record.

 

For a multi-record form, you should always explicitly assign an index for each "group" of records. If you are editing records, then set the index for each input to the record ID.

<input type="checkbox"<?php echo "$active"; ?>name="active[<?php echo $id; ?>]" />
<input type="text" style="padding: 0px;" name="name[<?php echo $id; ?>]" value="<? echo "$name"; ?>" />
<input type="file" name="menu_file[<?php echo $id; ?>]" />

 

Now, you do not need a hidden field to store the record ID, because you will have it as the array index for each value! Also, just an FYI, you are creating the field for "MAX_FILE_SIZE" in your loop. You only need it once on your form, so create it outside the loop.

 

Now, in your processing logic you want to go over each "group" of inputs. Since the unchecked checkboxes are not sent in the POST data, you can't use those. But, all the text inputs will be sent in the POST data even if they are empty. So, you can use that array to loop over each record set like this:

foreach($_POST['name'] as $id => $name)
{
    $active = (isset($_POST['active'])) ? 'on' : 'off' ;

    $file_name = $menu_file["name"]["$id"]; //name of file on users machine
    $file_type = $menu_file["type"]["$id"]; //type of file being uploaded
    $file_size = $menu_file["size"]["$id"]; //size of the uploaded file in bytes
    $file_error = $menu_file["error"]["$id"]; //returned PHP error codes for upload
    $file_temp = $menu_file["tmp_name"]["$id"]; //temporary name on server

    //Process the record data and VALIDATE

}

 

Now, instead of running the UPDATE query on each loop, you can use the data to prepare a single query to update all the records.

foreach($_POST['name'] as $id => $name)
{

    //All procesign and validation has been done prior to this
    $activeUpdates   .= "    WHEN {$id} THEN {$active}\n";
    $menuUpdates     .= "    WHEN {$id} THEN {$name}\n";
    $locationUpdates .= "    WHEN {$id} THEN {$upload_name}\n";

}

//Loop is complete, create ONE query to update all records
$query = UPDATE menus
         SET active = CASE
             {$activeUpdates}
             ELSE active
         END,

         SET menu = CASE
             {$menuUpdates}
             ELSE active
         END,

         SET location = CASE
             {$locationUpdates}
             ELSE active
         END";

Link to comment
Share on other sites

Thank you for your suggestions, I apologise if I came across as ungrateful..

 

 

I will probably take on your suggestion to allow the user to update individual rows on a separate page.

 

 

When thinking about it, it does make more sense, for the sake of the checkboxes, I might replace them with a drop down so a value is received either way.

 

 

George.

Link to comment
Share on other sites

When thinking about it, it does make more sense, for the sake of the checkboxes, I might replace them with a drop down so a value is received either way.

 

If the value is a boolean (i.e. True/False) a checkbox is the appropriate input. The first solution I showed above is very simple to implement when you want to allow the user to ONLY update that one field for multiple records. And, if they are editing multiple fields on a single record, then you just set the value to true/false based upon whether the checkbox field isset() in the post data.

Link to comment
Share on other sites

Thank you for your support,

 

 

Would you say this is now a more satisfactory, and clean way to code what is needed?

 

 

The reason my code was messy was probably down to not knowing an ability to reference an ID in an array name on a form.

 

 

 

<?php


/* 


This script is for use on the Manage Menu's pages and updates all menu items.


*/


/*==================================*/
/* DATE BASE CONFIGURATION			*/
/*==================================*/


include_once "../../config/config.php";


/*==================================*/
/* SET VARIABLES					*/
/*==================================*/


$id = $_POST["id"];
$active = $_POST["active"];
$menu_name = $_POST["name"];
$menu_file = $_FILES['menu_file'];
$i = "0";


/*==================================*/
/* SECURITY MEASURES				*/
/*==================================*/


array_map('mysql_real_escape_string', $menu_name);


/*==================================*/
/* Update Menu Names				*/
/*==================================*/


foreach($menu_name as $id => $name) {


//Variables for file upload
    $file_name = $menu_file["name"]["$id"]; //name of file on users machine
    $file_type = $menu_file["type"]["$id"]; //type of file being uploaded
    $file_size = $menu_file["size"]["$id"]; //size of the uploaded file in bytes
    $file_error = $menu_file["error"]["$id"]; //returned PHP error codes for upload
    $file_temp = $menu_file["tmp_name"]["$id"]; //temporary name on server

    //If empty menu name, forward to error.
if(empty($name)) { header("Location: ../../../index.php?page=Menu's"); $continue="0"; }

//Is a file upload needed?
if(isset($file_name) && $continue!="0") { 

	//Include handler.php, for file validation and upload
	include_once "./hanlder.php"; 

	//Update database
	$query = "UPDATE menus SET menu='$name', location='$file_location' WHERE id='$id'";
	$query = mysql_query($query) or die(mysql_error());	

}

//If a file upload is not required, update database excluding file location.
elseif(!isset($file_name) && $continue!="0") {
	$query = "UPDATE menus SET menu='$name' WHERE id='$id'";
	$query = mysql_query($query) or die(mysql_error());	
}

}


/*==================================*/
/* Update Active Status				*/
/*==================================*/


//Update active status if all validation and earlier tasks have completed.
if($continue!="0") {

//Convert to comma separated string
$ids = implode(',', $active);

//Create and run one query to update checked records
$query = "UPDATE menus SET active = 1 WHERE id IN ($ids)";
mysql_query($query) or die(mysql_error());

//Create and run one query to update unchecked records
$query = "UPDATE menus SET active = 0 WHERE id NOT IN ($ids)";
mysql_query($query) or die(mysql_error());

}


/*==================================*/
/* END OF SCRIPT					*/
/*==================================*/


header("Location: ../../../index.php?page=Menus&errors=Menu_Change_Success");


?>

Link to comment
Share on other sites

Cleaner code! You will now be able to work with this file a-lot easier.

 

<?php

include_once dirname(__FILE__) . '/config/config.php';

$id = $_POST['id'];
$active = $_POST['active'];
$name = $_POST['name'];
$menu_file = $_FILES['menu_file'];
$i = 0;

foreach($id as $key) {

    if(empty($active[$i])) {
        $active[$i] = 'off';
    }

    $active_post = $active[$i];
    $name_loop = $name[$i];
    $file_name = $menu_file['name'][$i];
    $file_type = $menu_file['type'][$i];
    $file_size = $menu_file['size'][$i];
    $file_error = $menu_file['error'][$i];
    $file_temp = $menu_file['tmp_name'][$i];

    if(!empty($file_name)) {

        if($file_type !== 'application/pdf') {
            header('Location: ' .  dirname(__FILE__) . '/index.php?page=Menus&error=Incorrect_File_Type');
            exit;
        } else {

            if($file_size > 5242880) {
                header('Location: ' .  dirname(__FILE__) . '/index.php?page=Menus&error=Incorrect_File_Size');
                exit;
            } else {
                $today = date('d-m-Y');
                $uploaddir = '/home/penmillh/en/includes/menus/';
                $upload_name = str_replace(' ', '', $name_loop) . $today . '.pdf';
                $uploadfile = $uploaddir . $upload_name;

                if($file_error !== 'UPLOAD_ERR_OK') {
                    header('Location: ' .  dirname(__FILE__) . '/index.php?page=Menus&error=File_Upload_Error');
                    exit;
                } elseif($file_error === 'UPLOAD_ERR_OK') {
                    move_uploaded_file($file_tmp, $uploadfile);

                    // You may want to concider a LIMIT in this query? Also, escape them inputs!!
                    $query = "UPDATE `menus` SET `active` = '" . $active . "', `menu` = '" . $name_loop . "', `location` = '" . $upload_name . "' WHERE `id` = '" . $value . "'";
                    $query = mysql_query($query) or die(mysql_error());

                    header('Location: ' .  dirname(__FILE__) . '/index.php?page=Menus&error=File_Upload_Success');
                    exit;
                }
            }
        }
    }

    if(empty($file_name)) {
        // Again, you may want a LIMIT here? And you need to escape them inputs!!
        $query = "UPDATE `menus` SET `active` = '" . $active_post . "' WHERE `id` = '" . $key . "'";
        $query = mysql_query($query) or die(mysql_error());

        /*
         * header('Location: ' .  dirname(__FILE__) . '/index.php?page=Menus&error=Menu_Change_Success');
         */
    }

    $i++;
}
?>

 

James.

 

EDIT: Forgot to add the

dirname(__FILE__)

.

Link to comment
Share on other sites

Personally, I don't like some of the changes jamesxg1 made. For example, I see no reason to define strings with double quotes and then exit the string to concatenate a variable. The beauty of double quotes strings is that you can have variables int he string that will be interpreted.

 

Anyway, here's my revision and some notes:

 

1. You were definging $id at the top of the page as

$id = $_POST["id"];

But, before you use that value, it gets overwritten when it gets redefined in the foreach() loop. That line should be removed.

 

2. You are not validating the values of active before using them in a query

 

3. The values for 'name' should also be trimmed and empty values discarded. No need to implement that before the foreach loop though.

 

4. You are already updating all the records in a loop, so there is no reason to handle the 'active' values separately. You should handle all your updates in one query, but if you are going to loop over each one do all the updates for each.

 

5. There is no reason to create two completely different queries based upon whether a file upload is done or not. Simply add logic to modify the one query as needed.

 

6. There was no validation of the $id as an integer.

 

The following should accomplish the same as you previously had, but is greatly simplified

<?php

/* 
This script is for use on the Manage Menu's pages and updates all menu items.
*/

/*==================================*/
/* DATE BASE CONFIGURATION			*/
/*==================================*/
include_once "../../config/config.php";

/*==================================*/
/* SET VARIABLES					*/
/*==================================*/
$menu_file = $_FILES['menu_file'];

/*==================================*/
/* Update Menu Names				*/
/*==================================*/
foreach($menu_name as $id => $menu_name)
{
    //Validate the id as an int
    if($id != intval($id))
    {
        header("Location: ../../../index.php?page=Menu's");
        exit();
    }

    //Validate record name
    $menu_name = mysql_real_escape_string(trim($menu_name));
    if(empty($menu_name))
    {
        header("Location: ../../../index.php?page=Menu's");
        exit();
    }

    //Set active value for record
    $active = (isset($_POST['active'][$id])) ? 1 : 0;

//Variables for file upload
    $file_name  = $menu_file["name"]["$id"]; //name of file on users machine
    $file_type  = $menu_file["type"]["$id"]; //type of file being uploaded
    $file_size  = $menu_file["size"]["$id"]; //size of the uploaded file in bytes
    $file_error = $menu_file["error"]["$id"]; //returned PHP error codes for upload
    $file_temp  = $menu_file["tmp_name"]["$id"]; //temporary name on server

    //Is a file upload needed?
    if(isset($file_name))
    { 
        //Include handler.php, for file validation and upload
        include_once "./hanlder.php"; 
    }

    //Update database
    //Only include change to location if '$file_location' is set
    $locationChange = (isset($file_location)) ? ", location='{$file_location}'" : '';
    $query = "UPDATE menus SET menu='{$name}', active={$active} {$locationChange} WHERE id='{$id}'";
    $query = mysql_query($query) or die(mysql_error());	
}

header("Location: ../../../index.php?page=Menus&errors=Menu_Change_Success");

?>

 

Lastly, defining the variables for the file and then calling an include() file is not efficient. You should not call the same include() multiple times since the server has to read that file into memory again and again. Instead, create a function to do the processing of the upload. You could still put that function into an include file, but you would only include it once in the page (before the loop). Also, instead of defining the variables for the file, I would create the function so that you only need to pass the file POST array ($_FILES['menu_file']) and the id to use. Then have that function return either the file location (if the file existed in the POST data and there were no error in the upload) or false. Then in the main script set the value of $file_location to the result of that function call:

$file_location = file_upload_function($_FILES['menu_file'], $id);

 

Then change these lines

    $locationChange = (isset($file_location)) ? ", location='{$file_location}'" : '';
    $query = "UPDATE menus SET menu='{$name}', active={$active} {$locationChange} WHERE id='{$id}'";

 

To this

    $locationChange = ($file_location!==false) ? ", location='{$file_location}'" : '';
    $query = "UPDATE menus SET menu='{$name}', active={$active} {$locationChange} WHERE id='{$id}'";

Link to comment
Share on other sites

Would you be able to explain what the ? does in a query?

 

I'm not using "?" in the query. I think you are referring to my use of the ternary operator. This is sometimes referred to as the shorthand version of if/else. But, it's not quite the same.

 

So, in this example

$locationChange = (isset($file_location)) ? ", location='{$file_location}'" : '';

 

I am setting the value of $locationChange based upon whether the variable $file_location is set. If it is set, then $locationChange will be set to the string: "", location='{$file_location}'"". If it is not set then $locationChange will be an empty string.

 

In a more broad sense it can be visualized like this

$variable = ( CONDITION[s] )  ?  TRUE_RESULT  :  FALSE_RESULT  ;

 

You can also use it for other purposes

echo "Today is a " . ((date('D')=='Sat' || date('D')=='Sun') ? "weekend" : "weekday"); 

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.