[murphey]

I've inherited a middling-size PHP/javascript/MySql system and have been cleaning it up for some time. There's one section of code (drop-down menu for dates) that baffles me.

<select name="expMonth">
    <option value="01"<?if($_POST['expMonth']=="01"){?> selected<?}?>>January</option>
    <option value="02"<?if($_POST['expMonth']=="02"){?> selected<?}?>>February</option>
    <option value="03"<?if($_POST['expMonth']=="03"){?> selected<?}?>>March</option>
</select>

I don't understand

1) why each line has "selected" by itself and not the expected

selected="selected"

and 2) why each line has this code snippet at all.

The objective is to get rid of all the drop-downs and use a function instead of replicating the gory details over and over (there are 12 places this is used) but until I understand this, I can't write the correct function.

Any ideas?

This post has been edited by no2pencil: 19 October 2012 - 08:15 PM
Reason for edit:: Added Code Tags

[creativecoding]

1. You can just use SELECTED. That's what I prefer, anyways.

2. It looks as if the original programmer was trying to convert the month from it's number format to the spelled equivalent (IE "12" == "December"). Not only is this a bad way of doing things, but it's also very poorly written. This snippet will return the month's name, where $month is the month's number:

date('F', mktime(0, 0, 0, $month));

EDIT:

Oh, it looks like you're trying to make the code smaller... Anyways, you can use this function:

function makeExpMonthList($month){
	$result = '<select name="expMonth">';
	for($i=0; $i<=12; $i++){
		$selected = ($month == $i) ? ' SELECTED' : '';
		$result .= '<option value="' . $i . '"' . $selected . '>' . date('F', mktime(0, 0, 0, $i)) . '</option>';
	}
	$result .= '</select>';
	return $result;
}

This post has been edited by creativecoding: 19 October 2012 - 08:39 PM

[indrora]

selected="selected"

Such an XHTML-ism that doesn't make any sense. It is perfectly valid to say

<option value="xx" selected>my text</option>

As for the rest, it sounds like someone didn't care to use a loop.

[murphey]

creativecoding, on 19 October 2012 - 08:30 PM, said:

1. You can just use SELECTED. That's what I prefer, anyways.

2. It looks as if the original programmer was trying to convert the month from it's number format to the spelled equivalent (IE "12" == "December"). Not only is this a bad way of doing things, but it's also very poorly written. This snippet will return the month's name, where $month is the month's number:

date('F', mktime(0, 0, 0, $month));

EDIT:

Oh, it looks like you're trying to make the code smaller... Anyways, you can use this function:

function makeExpMonthList($month){
	$result = '<select name="expMonth">';
	for($i=0; $i<=12; $i++){
		$selected = ($month == $i) ? ' SELECTED' : '';
		$result .= '<option value="' . $i . '"' . $selected . '>' . date('F', mktime(0, 0, 0, $i)) . '</option>';
	}
	$result .= '</select>';
	return $result;
}

Thank you for the better use of Month, will add that to the program. However I was using this only as the simplest and quickest example. There are numerous places in the code where using a function with a pre-defined array is easier and better than specifying every option line in the code.

A bit of background - this started many years ago as a senior class project (no, I wasn't the instructor. if I had been, the students would have flunked the project for crappy code, but they were beginners afterall) which someone thought was fantastic for a very small company. Functionality has grown and I'm trying to mitigate some of the original problems.

A good example of what I'm trying to eliminate is 4 identical drop-downs with over 100 items. Put the items in an array, invoke using the function, and life is so much simpler.

So, I've learned today (and thank you all, it's always good to learn something new) that I don't need the selected="selected" phrase. But I'm still baffled why each option has SELECTED. I was under the impression that only 1 SELECTED was allowed, and that was the default if the used didn't choose another option from the drop-down.

I've got this working as a stand-alone test case, but the selected value doesn't seem to be retained in the $_POST array for later use in the production code and written to the database.

[Xupicor]

Each option has selected in output HTML only if $_POST['expMonth'] equals "01", "02", "03", etc - all at the same time.

This post has been edited by Xupicor: 20 October 2012 - 11:59 AM

[murphey]

Xupicor, on 20 October 2012 - 11:58 AM, said:

Each option has selected in output HTML only if $_POST['expMonth'] equals "01", "02", "03", etc - all at the same time.

Sorry, I don't understand the explanation.

[creativecoding]

Because this code is poorly written.. Here, lemme rewrite it for you so you may understand better:

echo '<select name="expMonth">';
echo '  <option value="01"';
if($_POST['expMonth'] == "01"){
    echo " SELECTED";
}
echo '>';
echo '  <option value="02"';
if($_POST['expMonth'] == "02"){
    echo " SELECTED";
}
echo '>';
echo '  <option value="03"';
if($_POST['expMonth'] == "03"){
    echo " SELECTED";
}
echo '>';
echo '</select>';

This code will result in the same exact output. It's still not good code, but now you can see why SELECTED isn't always outputted.

[murphey]

creativecoding, on 20 October 2012 - 06:31 PM, said:

Because this code is poorly written.. Here, lemme rewrite it for you so you may understand better:

echo '<select name="expMonth">';
echo '  <option value="01"';
if($_POST['expMonth'] == "01"){
    echo " SELECTED";
}
echo '>';
echo '  <option value="02"';
if($_POST['expMonth'] == "02"){
    echo " SELECTED";
}
echo '>';
echo '  <option value="03"';
if($_POST['expMonth'] == "03"){
    echo " SELECTED";
}
echo '>';
echo '</select>';

This code will result in the same exact output. It's still not good code, but now you can see why SELECTED isn't always outputted.

Humble Grasshopper thanks you, one and all.

Solved that problem and got rid of about 1000 lines of code! *whew*

[CTphpnwb]

Here's how I'd do it:

<?php
$form = <<<MYFORM
<form method="post">
	<select name="expMonth">
		{options}
	</select>
	<input type="submit" value="Submit">
</form>
MYFORM;

echo str_replace("{options}", set_options(), $form);

function set_options() {
	$choice = '<option value="{val}"{sel}>{val}</value>'."\n\t\t";
	$options = '';
	$tags = array("{val}", "{sel}");
	if(isset($_POST['expMonth'])) {
		$n = (int)$_POST['expMonth'];
	} else {
		$n = 0;
	}
	for($i = 1; $i < 13; $i++) {
		if($n == $i) {
			$sel = " SELECTED";
		} else {
			$sel = '';
		}
		$options .= str_replace($tags, array($i,$sel), $choice);
	}
	return substr($options, 0, -3);
}

Actually, I'd read $form from a file, but the idea is: HTML is separated from PHP.