form method post only returning last input value

chrisdee

Hello.

I have made a form that should accept user input and then return/echo the input values to the user. For some reason it only returns values from the last input field. Apreciate if someone could tell me what Im doing wrong.

Here is the script :

<?php
$data = array('filename','date','size');
foreach($data as $values) {
 $values = $_POST[$values];
}

if (empty($_POST)) {
 echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
 echo "<table border=1>";
 foreach($data as $values) {
  echo "<tr><td>$values : <input type='text' name='".$values."'></td></tr>";
 }
 echo "</table>";
 echo "<input type='submit' value='Insert'>";
 echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
 echo "</form>";
}
else {
  echo $values;
}
?>

bradgrafelman

Your problem is here:

foreach($data as $values) { 
$values = $_POST[$values]; 
}

In this loop, you're constantly overwriting the value of the $values variable until the last iteration of the loop.

Instead, either build an [man]array[/man] of values, or simply move that loop down below to your echo statement and instead directly output the relevant $_POST indexes.

Also note that you should always verify that external data exists before you attempt to access it (e.g. by using [man]isset/man or [man]empty/man); otherwise, if it doesn't, you'll be generating E_NOTICE level error messages each time.

chrisdee

Thanks.

Looks like I found a solution. Not shure why it works but it does.:o

<?php
$data = array('filename','dato','size');
foreach($data as $values) {
 $values = $_POST[$values];
 $val[] = $values;
}

if (empty($_POST)) {
 echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
 echo "<table border=1>";
 foreach($data as $values) {
  echo "<tr><td>$values : <input type='text' name='".$values."'></td></tr>";
 }
 echo "</table>";
 echo "<input type='submit' value='Insert'>";
 echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
 echo "</form>";
}
else {
 foreach($val as $v) {
  echo $v."<br>";
 }
}
?>

sneakyimp

You really need to take a step back and try to understand what foreach does. This is a really silly piece of code:

foreach($data as $values) {
$values = $_POST[$values];
}

it says "set the $values var to each of the values in $data, one at a time, and after you set it to each item in $data, then just set it to whatever's in $POST[$values]". It accomplishes nothing except that when the loop completes, $values will be set to $POST['size']. Also, it's bad form to redefine either variable in your foreach statement in the loop itself.

Try reading the documentation on [man]foreach[/man].

chrisdee

How about this. Does it look better ?

<?php
$data = array('filename','date','size');

foreach($data as $key => $value) {
 $value = $_REQUEST[$value];
 $val[] = $value;
}

if (empty($_POST)) {
echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
echo "<table border=1>";
foreach($data as $key => $value) {
  echo "<tr><td>" . $key . " : <input type='text' name='" . $value . "'></td></tr>";
}
echo "</table>";
echo "<input type='submit' value='Insert'>";
echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
echo "</form>";
}
else {
 echo "<pre>";
 print_r($val);
 echo "</pre>";
}
?>

chrisdee

Sorry. Dont look at the previous code, but rather at the cleaned up code below.
Does it look ok ?

<?php
$data = array('filename','date','size');

foreach($data as $key => $value) {
 $val[] = $_REQUEST[$value];
}

if (empty($_POST)) {
echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
echo "<table border=1>";
foreach($data as $key => $value) {
  echo "<tr><td>" . $data[$key] . " : <input type='text' name='" . $value . "'></td></tr>";
}
echo "</table>";
echo "<input type='submit' value='Insert'>";
echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
echo "</form>";
}
else {
 echo "<pre>";
 print_r($val);
 echo "</pre>";
}
?>

sneakyimp

Yes it's looking better. Is it doing what you want?

Derokorian

chrisdee;10998893 wrote:

Sorry. Dont look at the previous code, but rather at the cleaned up code below.
Does it look ok ?

<?php
$data = array('filename','date','size');

foreach($data as $key => $value) {
 $val[] = $_REQUEST[$value];
}

if (empty($_POST)) {
echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
echo "<table border=1>";
foreach($data as $key => $value) {
  echo "<tr><td>" . $data[$key] . " : <input type='text' name='" . $value . "'></td></tr>";
}
echo "</table>";
echo "<input type='submit' value='Insert'>";
echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
echo "</form>";
}
else {
 echo "<pre>";
 print_r($val);
 echo "</pre>";
}
?>

My biggest problem with this is inconsistent indentation. Some control structure's content is indented some aren't and it makes it harder to easily match up braces. In a short script like this not that big a deal, but when you have longer scripts and you miss a brace somewhere not indenting will only hurt you. Also I would put spacing around control structures (like you have for the first foreach), again to help you quickly separate the flow visually. As such I would have it like this (note: none of the code was changed, just the indentation):

<?php
$data = array('filename','date','size');

foreach($data as $key => $value) {
	$val[] = $_REQUEST[$value];
}

if (empty($_POST)) {
	echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
	echo "<table border=1>";

foreach($data as $key => $value) {
	echo "<tr><td>" . $data[$key] . " : <input type='text' name='" . $value . "'></td></tr>";
}

echo "</table>";
echo "<input type='submit' value='Insert'>";
echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
echo "</form>";
}
else {
	echo "<pre>";
	print_r($val);
	echo "</pre>";
}
?>

chrisdee

sneakyimp;10998909 wrote:
Yes it's looking better. Is it doing what you want?

Yes it is. Thank you for your help and feedback.
Much apreciated.