My first app... ...be nice!

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
masson
Forum Newbie
Posts: 4
Joined: Wed Jan 31, 2007 8:53 am

My first app... ...be nice!

Post by masson »

Well I thought it would be good for a little reflection. This is my first ever application, nothing fancy but I am learning. Lots of fun. I would applicate any comments, critiques, criticisms. Also thanks to the DevNetwork for all of the members who helped me out.

First a form (requesting travel expense reimbursements) is presented to the user...

Code: Select all

<html>
	<body>

<!-- Centered for style -->
	<center>

<!-- FORM 1: Interface for user input of travel information, expenses and contacts for reimbursement. Data collected is passed to "travelRequestPost.php" for accuracy and tabulation of expenses. -->
	<form action="travelRequestPost.php" method="post">

<!-- TABLE 1 START: Form composed of a 5 column, 20 row table -->
	<table>
<!-- TABLE 1: Rows 1 - 2, form title information -->
		<tr>
			<td colspan="5"><img src="./delhiLogo.jpg"></td>
		</tr>
		<tr>
			<td colspan="5" align="middle"><font face="arial"><b>T R A V E L &nbsp R E Q U E S T</b><br><br></font></td>
		</tr>
<!-- Row 3: Date & phone -->
		<tr>
			<td><font face="arial">Date of Request:</font></td>
			<td><font face="arial"><input type="text" size="10" name="date" /></font></td>
			<td></td>
			<td><font face="arial">Business Phone:</font></td>
			<td><font face="arial"><input type="text" size="10" name="bizPhone" /></font></td>
		</tr>
<!-- Row 4: Departure & return dates -->
		<tr>
			<td><font face="arial">Departure Date:</font></td>
			<td><font face="arial"><input type="text" size="10" name="depDate" /></font></td>
			<td></td>
			<td><font face="arial">Return Date:</font></td>
			<td><font face="arial"><input type="text" size="10" name="retDate" /></font></td></td>
		</tr>
<!-- Row 5: Name & zip code -->
		<tr>
			<td colspan="3"><font face="arial">Name: <input type="text" size="35" name="name" /></font></td>
			<td><font face="arial">Home Zip Code:</font></td>
			<td><font face="arial"><input type="text" size="10"name="homeZip" /></font></td>
		</tr>
<!-- Row 6: Campus ID & negotiating unit -->
		<tr>
			<td><font face="arial">Delhi 800 Number:</font></td>
			<td><font face="arial"><input type="text" size="11" name="ssn" /></font></td>
			<td></td>
			<td><font face="arial">Negotiating Unit:</font></td>
			<td><font face="arial"><input type="text" name="negUnit" /></font></td>
		</tr>
<!-- Row 7: Textarea for travel description -->
		<tr>
			<td colspan="5"><font face="arial">Purpose for Travel:<br><textarea name="purpose" rows="10" cols="80"></textarea></font></td>
		</tr>
<!-- Row 8: Transportation mode-->
		<tr>
			<td align="center" colspan="5">
				<font face="arial">
				Mode of transportation:
				<select name="transMode">
					<option value="State Car">State Car</option>
					<option value="Personal Car">Personal Car</option>
					<option value="Other Transporation">Other Transportation</option>
				</select>
				<br><br>
				License Number: <input type="text" size="11" name="license" /></font></td>
		</tr>
<!-- Row 9: Form directions -->
		<tr>
			<td align="center" colspan="5">
				<font face="arial"><hr>
   				To be competed by person making request and approved by authorizing travel.
				<br><br></font>
			</td>
		</tr>
<!-- Rows 10 - 14: Travel expenses. Below values are calculated at travelRequestPost.php and presented as "TOTAL EXPENSES" -->
		<tr>
			<td><font face="arial">Meals:</font></td>
			<td colspan="2"><font face="arial">$<input type="text" name="meals"></font></td>
			<td></td>
			<td></td>
		</tr>
		<tr>
			<td><font face="arial">Lodging:</font></td>
			<td colspan="2"><font face="arial">$<input type="text" name="lodging"></font></td>
			<td></td>
			<td></td>
		</tr>
		<tr>
			<td><font face="arial">Milage:</font></td>
			<td colspan="2"><font face="arial">$<input type="text" name="milage"></font></td>
			<td></td>
			<td></td>
		</tr>
		<tr>
			<td><font face="arial">Transportation:</font></td>
			<td colspan="2"><font face="arial">$<input type="text" name="trans"></font></td>
			<td></td>
			<td></td>
		</tr>
		<tr>
			<td><font face="arial">Miscellaneous:</font></td>
			<td colspan="2"><font face="arial">$<input type="text" name="misc"></font></td>
			<td></td>
			<td></td>
		</tr>
<!-- Row 15: Separator -->
		<tr>
			<td colspan="5"><br><hr><br></td>
		</tr>
<!-- Row 16: Account infomration -->
		<tr>
			<td colspan="3"><font face="arial">Charge to Line Number: <input type="text" size="10" name="lineItem"></font></td>
			<td colspan="2"><font face="arial">Amount Authorized: $<input type="text" name="amountAvail"></font></td>
		</tr>
<!-- Row 17: Separator -->
		<tr>
			<td colspan="5"><br><hr><br></td>
		</tr>
<!-- Rows 18 - 19: Supervisor information -->
		<tr>
			<td><font face="arial">Supervisor's Name:</font></td>
			<td colspan="4"><font face="arial"><input type="text" name="supName" size="25"></font></td>
		</tr>
		<tr>
			<td><font face="arial">Supervisor's E-mail:</font></td>
			<td colspan="4"><font face="arial"><input type="text" name="supEmail" size="25"></font></td>
		</tr>
<!-- Row 20: User's email -->
		<tr>
			<td><font face="arial">Your E-mail:</font></td>
			<td colspan="4"><font face="arial"><input type="text" name="yourEmail" size="25"></font></td>
		</tr>
<!-- TABLE 1 END -->
	</table>

	<br><br>

<!-- Form submission button -->
	<input type="submit" value="Review Form"/>

<!-- Form 1 END -->
	</form>

	</center>

	</body>
	</html>
Upon submission, the information entered is presented to the user for verification. If there is a error they can return to edit the information. They may also print out a hard copy.

Code: Select all

<html>
	<body>


<!-- Centered for style -->

	<center>

<!-- FORM 1: Interface for user input of travel information, expenses and contacts for reimbursement. Data collected is passed to "travelRequestPost.php" for accuracy and tabulation of expenses. -->

	<form  action="travelRequestMail.php" method="post">

<!-- Creates file name for travel request information -->
		<input type="hidden" name="fileName" value="<?php echo $_POST["ssn"];echo date("-Y-m-d-H-i-s"); ?>.txt" /> 

<!-- TABLE 1 START: Composed of a 5 column, 20 row table that prints output from travelRequest.php -->
	<table width=640>
<!-- TABLE 1: Row 1 - 2, form title information -->
		<tr>
			<td><img src="./delhiLogo.jpg"></td>
		</tr>
		<tr>
			<td colspan="5" align="middle"><font face="arial"><b>T R A V E L &nbsp R E Q U E S T</b><br><br></font></td>
		</tr>
<!-- Row 3: Print date & phone -->
		<tr>
			<td><font face="arial">Date of Request:</font></td>
			<td><font face="arial"><?php echo $_POST["date"]; ?></font></td>
			<td></td>
			<td><font face="arial">Business Phone:</font></td>
			<td><font face="arial"><?php echo $_POST["bizPhone"]; ?></font></td>
		</tr>
<!-- Row 4: Print departure & return dates -->
		<tr>
			<td><font face="arial">Departure Date:</font></td>
			<td><font face="arial"><?php echo $_POST["depDate"]; ?></font></td>
			<td></td>
			<td><font face="arial">Return Date:</font></td>
			<td><font face="arial"><?php echo $_POST["retDate"]; ?></font></td></td>
		</tr>
<!-- Row 5: Print user's name & home zip code -->
		<tr>
			<td><font face="arial">Name:</font></td>
			<td colspan="2"><font face="arial"><?php echo $_POST["name"]; ?></font></td>
			<td><font face="arial">Home Zip Code:</font></td>
			<td><font face="arial"><?php echo $_POST["homeZip"]; ?></font></td>
		</tr>
<!-- Row 6: Print user's SSN or Delhi 800 ID & negotiating unit -->
		<tr>
			<td><font face="arial">Delhi 800 Number:</font></td>
			<td><font face="arial"><?php echo $_POST["ssn"]; ?></font></td>
			<td></td>
			<td><font face="arial">Negotiating Unit:</font></td>
			<td><font face="arial"><?php echo $_POST["negUnit"]; ?></font></td>
		</tr>
<!-- Row 7: Print travel discussion -->
		<tr>
			<td colspan="5"><font face="arial"><hr>Purpose for Travel:<br><?php echo $_POST["purpose"]; ?></font></td>
		</tr>
<!-- Row 8: Print travel mode & and license number -->
		<tr>
			<td colspan="5">
				<font face="arial"><hr>
					Transportation: <?php echo $_POST["transMode"]; ?><br><br>
					License Number: <?php echo $_POST["license"]; ?>
				</font>
			</td>
		</tr>
<!-- Row 9: Form instructions -->
		<tr>
			<td align="center" colspan="5">
				<font face="arial"><hr>
					To be competed by person making request and approved by authorizing travel.<br><br>
				</font>
			</td>
		</tr>
<!-- Row 10: Print user's meal expenses in dollars -->
		<tr>
			<td><font face="arial">Meals:</font></td>
			<td colspan="2"><font face="arial">$<?php echo $_POST["meals"]; ?></font></td>
			<td></td>
			<td></td>
		</tr>
<!-- Row 11: Print user's lodging expenses in dollars -->
		<tr>
			<td><font face="arial">Lodging:</font></td>
			<td colspan="2"><font face="arial">$<?php echo $_POST["lodging"]; ?></font></td>
			<td></td>
			<td></td>
		</tr>
<!-- Row 12: Print user's milage expenses in dollars -->
		<tr>
			<td><font face="arial">Milage:</font></td>
			<td colspan="2"><font face="arial">$<?php echo $_POST["milage"]; ?></font></td>
			<td></td>
			<td></td>
		</tr>
<!-- Row 13: Print user's additional transporation expenses in dollars -->
		<tr>
			<td><font face="arial">Transportation:</font></td>
			<td colspan="2"><font face="arial">$<?php echo $_POST["trans"]; ?></font></td>
			<td></td>
			<td></td>
		</tr>
<!-- Row 14: Print user's miscellaneous expenses in dollars -->
		<tr>
			<td><font face="arial">Miscellaneous:</font></td>
			<td colspan="2"><font face="arial">$<?php echo $_POST["misc"]; ?></font></td>
			<td></td>
			<td></td>
		</tr>
<!-- Row 15: Print total of all expenses in dollars -->
		<tr>
			<td><font face="arial">TOTAL EXPENSES:</font></td>
			<td colspan="2">
				<font face="arial">
					$<?php echo $_POST["meals"]+$_POST["lodging"]+$_POST["milage"]+$_POST["trans"]+$_POST["misc"]; ?>
				</font>
			</td>
			<td></td>
			<td></td>
		</tr>
<!-- Row 16: Separator -->
		<tr>
			<td colspan="5"><br><hr><br></td>
		</tr>
<!-- Row 17: Print account information -->
		<tr>
			<td colspan="3"><font face="arial">Charge to Line Number: <?php echo $_POST["lineItem"]; ?></font></td>
			<td colspan="2"><font face="arial">Amount Authorized: $<?php echo $_POST["amountAvail"]; ?></font></td>
		</tr>
<!-- Row 18: Separator -->
		<tr>
			<td colspan="5"><br><hr><br></td>
		</tr>
<!-- Row 19: Separator -->
		<tr>
			<td><font face="arial">Supervisor's Name:</font></td>
			<td colspan="4"><font face="arial"><?php echo $_POST["supName"]; ?></font></td>
		</tr>
<!-- Rows 20 - 21: Supervisor information -->
		<tr>
			<td><font face="arial">Supervisor's E-mail:</font></td>
			<td colspan="4">
				<font face="arial">
					<?php echo $_POST["supEmail"]; ?><input type="hidden" name="toEmail" value="<?php echo $_POST["supEmail"]; ?>" />
				</font>
			</td>
		</tr>
		<tr>
			<td><font face="arial">Your E-mail:</font></td>
			<td colspan="4">
				<font face="arial">
					<?php echo $_POST["yourEmail"]; ?><input type="hidden" name="frEmail" value="<?php echo $_POST["yourEmail"]; ?>" /> 
				</font>
			</td>
		</tr>
<!-- Row 22: Separator -->
		<tr>
			<td colspan="5"><br><hr><br></td>
		</tr>
<!-- Row 23: Instructions for submission -->
		<tr>
			<td colspan="5">
				<font face="arial">
					<b>PLEASE REVIEW:</b><br><br>
					If the information above IS NOT correct you may 
<!-- Javascript to return user to editable form -->
					<a href="javascript:history.back()">return</a> to the Travel Request Form for editing.<br><br>

					If the information above is correct, you may forward this form via email to <?php echo $_POST["supName"]; ?> 
<!-- Javascript to print form -->
					or you may <a href="javascript:window.print()">print the form</a>.
				</font>
			</td>
		</tr>
<!-- TABLE 1 END -->
	</table>

<!-- Hides textarea from users -->
	<div style="display:none;">

<!-- Textarea formats travel request information for email -->
		<textarea name="requestInfo" ROWS=0 COLS=0>
Date of Request:  <?php echo $_POST["date"]; ?> 
Name:             <?php echo $_POST["name"]; ?> 
Delhi Phone:      <?php echo $_POST["bizPhone"]; ?> 
Delhi E-mail:     <?php echo $_POST["yourEmail"]; ?> 
Delhi 800 Number: <?php echo $_POST["ssn"]; ?> 
Home Zip Code:    <?php echo $_POST["homeZip"]; ?> 
Negotiating Unit: <?php echo $_POST["negUnit"]; ?> 
 
Travel dates:     From: <?php echo $_POST["depDate"]; ?>, to: <?php echo $_POST["retDate"]; ?> 
 
Purpose for Travel: 
<?php echo $_POST["purpose"]; ?> 
 
Transportation: <?php echo $_POST["transMode"]; ?> 
License Number: <?php echo $_POST["license"]; ?> 
 
Meals: $<?php echo $_POST["meals"]; ?> 
Lodging: $<?php echo $_POST["lodging"]; ?> 
Milage: $<?php echo $_POST["milage"]; ?> 
Trans: $<?php echo $_POST["trans"]; ?> 
Misc: $<?php echo $_POST["misc"]; ?> 
Total: $<?php echo $_POST["meals"]+$_POST["lodging"]+$_POST["milage"]+$_POST["trans"]+$_POST["misc"]; ?> 

Charge to Line Number: <?php echo $_POST["lineItem"]; ?>,   Amount Authorized: $<?php echo $_POST["amountAvail"]; ?> 
 
_____________BELOW FOR BUSINESS OFFICE APPROVAL_______________
Requisition Number:______________________________________________ 
Account No.____________________Object__________Amount $__________ 
Account No.____________________Object__________Amount $__________ 
Account No.____________________Object__________Amount $__________ 
Account No.____________________Object__________Amount $__________ 

		</textarea>
	</div>

	<br><br>

<!-- Form submission button -->
	<input type="submit" value="Email and Save Form"/>

<!-- Form 1 END -->
	</form>

	<br><br>

	</body>
	</html>
If all is good, the hit submit and the following file sends an email to their supervisor and writes a copy to the server.

Code: Select all

<?php


	$File = $_POST["fileName"];
	$Handle = fopen($File, 'w');
	$Data = $_POST["requestInfo"];
	fwrite($Handle, $Data);
	fclose($Handle);


// set empty variable
	$toEmail = "";

// Check if there are any values posted for $toEmail then set the variable passed from "travelRequestPost.php"
	if(isset($_POST["toEmail"]) && ($_POST["toEmail"] != NULL)){
	    $toEmail = $_POST["toEmail"];
	} 

// set empty variable
	$frEmail = "";

// Check if there are any values posted for $frEmail then set the variable passed from "travelRequestPost.php"
	if(isset($_POST["frEmail"]) && ($_POST["frEmail"] != NULL)){
		$frEmail = $_POST["frEmail"];
	} 

// set empty variable
	$requestInfo = "";

// Check if there are any values posted for $requestInfo then set the variable passed from "travelRequestPost.php"
	if(isset($_POST["requestInfo"]) && ($_POST["requestInfo"] != NULL)){
		$requestInfo = $_POST["requestInfo"];
	} 

// Declare email variables
	$to = $toEmail;
	$subject = "Travel Request";
	$message = $requestInfo;
	$from = $frEmail;
	$headers = "From: $from";
// Email function
	mail($to,$subject,$message,$headers);
	echo "<html><body><center><img src='./delhiLogo.jpg'><p>Your Travel request was sent to $to and is available <a href=$File>online</a>.<p>You may now return to <a href='http://www.delhi.edu'>the SUNY Delhi Homepage</a>.<p>If you have any questions please contact the CIS Help Desk.</center></body></html>";

?>
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

I see... <font> tags!
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Yes, on the HTML front you could start cleaning things up with this:

Code: Select all

<style type="text/css">
body, p, td {
     font-family: Arial, sans-serif;
     }
</style>
(#10850)
User avatar
califdon
Jack of Zircons
Posts: 4484
Joined: Thu Nov 09, 2006 8:30 pm
Location: California, USA

Post by califdon »

I didn't spend a lot of time examining your code, but I did spot two places where you are missing the semicolon at the end of &nbsp; and in the second script, you are missing the opening tag for the textarea.

It's okay to post a lot of code and ask for comments, but the real test is does it work?
masson
Forum Newbie
Posts: 4
Joined: Wed Jan 31, 2007 8:53 am

Thanks

Post by masson »

Yeah it works perfect. I have even added some more functionality.

I guess I was really interested in if things seemed efficient: a working hack vs. written well. The font tag issue was a good call, initially I didn't care as it will only be available to a few internal folks, not the public, so css wasn't an issue, but I like the suggestion made, so I modified the code: definitely more efficient.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

I think the next step would be using a template class and a simple HTML tag generator for the form fields. That would reduce having two copies of essentially the same HTML.
(#10850)
Post Reply