seeking more efficient code

[jswebdev]

please take a look at this script. i was having problems getting my <select><option> html form to work properly until i, against my better judgement, added the two additional mysql_fetch_array functions for "artist_id2" and "artist_id3" (support one, and support two).

do you have a suggestion for a more efficient way to write this code?
i have a snapshot of my table here. i hope you can understand in the midst of this mess what i'm trying to do...


thanks!!

Code: Select all

<?php
$conn = mysql_connect("localhost", "myuser", "mypass")
	or die(mysql_error());
mysql_select_db("cb_shows1", $conn) or die(mysql_error());


if ($_POST['op'] != "add") {
// TESTING MATCH FOR HIDDEN FOR FIELD ABOVE. IT WILL ALWAYS FAIL FIRST TIME SO THE FORM IS IN FACT DISPLAYED
	
		// GET PARTS OF RECORDS
	
	$get_artist_ids = "select artist_id, artist_name from artist_info order by artist_name";
	$get_artist_ids_res = mysql_query($get_artist_ids) or die(mysql_error());
	
	$artist_ids = $get_artist_ids_res;
	$artist_ids1 = $get_artist_ids_res;
			
// if LOGIC - CHECK FOR EXISTENCE OF CONCERT RECORDS.
	// NO RESULTS = DISPLAY SORRY NO RECORDS.
	// RECORDS = SHOW CONCERT RECORDS IN SELECTOR FORM.
	}
	
	
	
	if (mysql_num_rows($artist_ids) < 1) {
		// NO RECORDS
		$display_block .= "

Sorry, no artists exist to select!</p>
							

Please <a href=\"addartist.php\">go add the Artists first</a>, then come back and
							add the Concert Date</p>";
	} else {
	$display_block = "<h1>Add a Date:</h1>
	<form method=\"post\" action=\"$_SERVER[PHP_SELF]\" >
		
		

Headliner:
		

		<select name=\"artist_id1\">
			<option value=\"\">-- Select Headliner --</option>";

		while ($recs1 = mysql_fetch_array($artist_ids1)) {
			$artist_id1 = $recs1['artist_id'];
			$artist_name1 = $recs1['artist_name'];

			$display_block .= "<option value=\"$artist_id1\">
				$artist_name1</option>";
		}
		$display_block .= "</select>
		
		

First Supporting Act:
		

		<select name=\"artist_id2\">
			<option value=\"\">-- Select First Support --</option>";

	$get_artist_ids2 = "select artist_id, artist_name from artist_info order by artist_name";
	$get_artist_ids_res2 = mysql_query($get_artist_ids2) or die(mysql_error());
		
		while ($recs2 = mysql_fetch_array($get_artist_ids_res2)) {
			$artist_id2 = $recs2['artist_id'];
			$artist_name2 = $recs2['artist_name'];

			$display_block .= "<option value=\"$artist_id2\">".$artist_name2."</option>";
		}
		$display_block .= "</select>
		
		

Second Supporting Act:
		
		<select name=\"artist_id3\">
			<option value=\"\">-- Select Second Support --</option>";
	
	$get_artist_ids3 = "select artist_id, artist_name from artist_info order by artist_name";
	$get_artist_ids_res3 = mysql_query($get_artist_ids3) or die(mysql_error());
	
		while ($recs3 = mysql_fetch_array($get_artist_ids_res3)) {
			$artist_id3 = $recs3['artist_id'];
			$artist_name3 = stripslashes($recs3['artist_name']);

			$display_block .= "<option value=\"$artist_id3\">
				$artist_name3</option>";
		}
		$display_block .= "</select>
		

Show Date:
		

		<input type=\"text\" name=\"show_date\" size=\"20\" maxlength=\"25\" value=\"yyyy-mm-dd\" />
		</p>
		
		

Show Time:
		

		<input type=\"text\" name=\"show_time\" size=\"20\" maxlength=\"25\" value=\"HH:MM\" />
		</p>
		
		

Ticket Price Date of Show:
		

		<input type=\"text\" name=\"tix_dos\" size=\"20\" maxlength=\"25\" value=\"yyyy-mm-dd\" />
		</p>
		
		

Tickets in Advancee:
		

		<input type=\"text\" name=\"tix_adv\" size=\"20\" maxlength=\"25\" value=\"HH:MM\" />
		</p>
		
		
		

All Ages?
		

		
		<input type=\"radio\" value=\"yes\" checked=\"checked\" /> yes
		<input type=\"radio\" value=\"no\" /> no
		</p>
		
		<input type=\"hidden\" name=\"clear\" value=\"\" />
		
		

Alcohol Served?
		

		<input type=\"radio\" value=\"yes\" checked=\"checked\" /> yes
		<input type=\"radio\" value=\"no\" /> no
		</p>
		
		

Concert Date Notes:
		

		<textarea name=\"concert_notes\" cols=\"35\" rows=\"5\" wrap=\"virtual\">notes on this concert
		</textarea>
		</p>
		
		

		

		<input type=\"hidden\" name=\"op\" value=\"add\" />
		
		

<input type=\"submit\" name=\"submit\" value=\"Add Date\" />
		</p>
		
		</form>";

} 
if ($_POST['op'] == "add") {
	//CHECKS FOR FIELDS REQUIRED TO SUBMIT THE FORM. IF MISSING DATA, FORM WILL RELOAD
	if (($_POST['artist_id1'] == "") || ($_POST['show_date'] == "")) {
		header("Location: add_date.php");
		exit;
	}

	//	CONNECT TO DATABASE
	$conn = mysql_connect("localhost", "myuser", "mypass")
		or die(mysql_error());
	mysql_select_db("cb_shows1", $conn) or die(mysql_error());

	// ADD TO ARTIST_INFO TABLE
	$add_artist = "insert into concerts values ('', '$_POST[artist_id1]', '$_POST[artist_id2]', '$_POST[artist_id3]', now(), '', '$_POST[show_date]', '$_POST[show_time]', '$_POST[tix_dos]', '$_POST[tix_adv]', '$_POST[age_req]', '$_POST[alcohol]', '$_POST[concert_notes]')"; 
	$artist_id1 = $_POST['artist_id1'];
	$artist_id2 = $_POST['artist_id2'];
	$artist_id3 = $_POST['artist_id3'];
	$todayadd = date(mm-dd-yyyy);
	$show_date = $_POST['show_date'];
	$show_time = $_POST['show_time'];
	$tix_dos = $_POST['tix_dos'];
	$tid_adv = $_POST['tix_adv'];
	$age = $_POST['age_req'];
	$alcohol = $_POST['alcohol'];
	$concert_notes = $_POST['concert_notes'];
	mysql_query($add_artist) or die(mysql_error());
	
	$display_block = "<h1>Artist Entry Added</h1>
		

The following info was inserted into the Crowbar Concert Database - Artist Table:</p>
		<h3 class=\"stickout\">Artist ID1:</h3>
		

$artist_id1</p>
		<h3 class=\"stickout\">Artist ID2:</h3>
		

$artist_id2</p>
		<h3 class=\"stickout\">Artist ID3:</h3>
		

$artist_id3</p>
		<h3 class=\"stickout\">Today's Date:</h3>
		

$todayadd</p>
		<h3 class=\"stickout\">Show Date:</h3>
		

$show_date</p>
		<h3 class=\"stickout\">Show Time:</h3>
		

$show_time</p>
		<h3 class=\"stickout\">Tickets Date of Show:</h3>
		

$tix_dos</p>
		<h3 class=\"stickout\">Tickets in Advance:</h3>
		

$tid_adv</p>
		<h3 class=\"stickout\">Age Requirement?</h3>
		

$age</p>
		<h3 class=\"stickout\">Alcohol to be Served?</h3>
		

$alcohol</p>
		<h3 class=\"stickout\">Concert Notes:</h3>
		

$concert_notes</p>
		<p class=\"stickout\">Would you like to <a href=\"add_date.php\">add another</a> entry?</p>
		

View existing <a href=\"sel_concerts.php\">Table Data.</a></p>
		

View Specific Concert Date</p>
		

View <a href=\"control.htm\">All Options</a></p>";

}
?>
<html>
<head>
<title>Add an Entry</title>
<link rel="stylesheet" type="text/css" href="css/php.css" />
</head>
<body>
<?php
echo $display_block; ?>
</body>
</html>

[grahame]

I can see a couple of things that may help as starters...

Firstly, I see a lot of code that's been cut and pasted a number of times and the altered a bit. If you find your self cutting and pasting code to write a script - there's probably an easier way ... and you're making yourself a maintenance nightmare if you insist on duplicating code

Don't copy and paste - write a function. If you have to change bits of the code when you copy and paste, THAT is the clue as to what should be passed into the function as parameters

Second - Don't write long, multiline quotes strings with backslashed quotes within. There are several alternatives, including a here document.

Code: Select all

$display_block .=  <<<DONE
This is a block of text to print
that is several lines long
DONE;

[jswebdev]

grahame,

first off, THANK you very much for the tip on functions()


i've been studying PHP for a little while, but only started keeping a log of notes to myself, and that little bit is going in there!

now, for the other part-- your recommendation about the long parts of multiple strings... i don't understand what you're telling me, but i'd like to learn. thanks so much!!

Originally Posted by grahame

Don't write long, multiline quotes strings with backslashed quotes within. There are several alternatives, including a here document.

Code: Select all

$display_block .=  <<<DONE
This is a block of text to print
that is several lines long
DONE;

[grahame]

In a quoted string as shown in your original sample code, you had spent a great deal of time adding \ characters in front of any \ you really wanted within the string. Not only had you taken (I'm sure) quite a while to get those all correct, but it also made reading the script much harder.

By using a "here document" - perhaps that would be better if it was called a multicharacter delimiter - you can make the code look much easier. The delimiter can be more or less anything - I chose the word DONE but you'll often find people using something more descriptive like HEADER or even EOT (for end of text).

Example showing (almost) the same text both ways:

Code: Select all

#!/usr/bin/php -q

<?php
$whom = "Graham";

print "\"This is a piece
of text over many lines\" says
$whom \"and I'm being very 
careful to balance my quotes\"\n";

print <<<BETTER
"This is a piece
of text over many lines" says
$whom "and I'm not having to be
careful to balance my quotes"\n
BETTER;
?>

[jswebdev]

oh! i see-- so, that's just done like this where ever i want echo (or print), or put it in a variable:

Code: Select all

echo <<<DELIMITERWORD 
a whole lot of text in here and i can use html code too? the end 
DELIMITERWORD

$variable = "

And here's even a whole lot more to look at.</p>



Send your email address here to show that you visited the link above</p>
<form method="post" action="thanks.php">


<input type="text" name="email" value="email_add" /></p>


<input type="submit" name="submit" value="send_email" /></p>
</form>"

is that basically how it's used?

thanks!!