Jump to content

The way PHP/MySQL should be coded?


Recommended Posts

So I've seen PHP and MySQL by myself only, noone helped and no school courses. I've been following examples and tutorials online and a book. What do you guys think about this script?This script is for a page where users can add links to the DB. Of course this is a logged in area. I added some comments for you guys.

<?php session_start(); ob_start(); //used to change header without errors ?><?php if(isset($_SESSION['userid'])){ //Check if user is logged in ?><?php if(isset($_POST['cancel'])){ //If the cancel button is pressed in form, user redirected to acp page.	header("Location: acp.php");	exit();} ?><?php include('../includes/mf/header.php'); ?>	<div class="fullcontent">		<h1>Add Link</h1>		<?php if (isset($_POST['add'])) {			//Initialize error array			$errors = array();						//Validate name			if(empty($_POST['sitename'])){				$errors[] = "<p class=\"centerAlign\">Site name field is required.</p>";			} else {				$n = trim($_POST['sitename']);			}						//Validate address			if(empty($_POST['siteurl'])){				$errors[] = "<p class=\"centerAlign\">Site URL field is required.</p>";			} else {				$n = trim($_POST['siteurl']);			}								//If everything is OK			if(empty($errors)){				//Insert feedback in database				require_once('../includes/mysql_connect.php');							//Query the database				$query = "INSERT INTO mf_links (sitename, siteurl, dateadded) VALUES('" . $_POST['sitename'] . "','" . $_POST['siteurl'] . "', NOW() )";								//Run query				$result = @mysql_query($query);							//If it ran OK				if($result){					header("Location: acp.php");					exit();				} else {					echo "<h3>Your new link was not submitted due to a system error.</h3>";					echo "<p>" . mysql_error() . "<br><br>Query:" . $query . "</p>";					include('../includes/mf/footer.php');					exit();				}				mysql_close();			} else {				for($i=0 ; $i<sizeof($errors) ; $i++){					echo $errors[$i]; //display errors				}			}		} ?>		<form name="addlink" method="post" action="add.php" class="form_addlink">			<div class="form_rowspacing"><label>Site Name</label> <input type="text" name="sitename" class="text_input" /></div>			<div class="form_rowspacing"><label>Site URL</label>  http://<input type="text" name="siteurl"  class="text_input" /></div>			<hr />			<input type="submit" name="add" value="Add Link" />			<input type="submit" name="cancel" value="Cancel" />		</form>	</div><?php include("../includes/mf/footer.php"); ?><?php } else { //If user is not logged,he is redirected	header("Location: index.php");	exit();} ?>

If you guys have any comments or suggestions about my php/mysql coding go ahead.

Link to post
Share on other sites

It's generally pretty good, you format it so it's easy to read. The one thing I would change is to group your PHP statements, there's no reason to close the PHP engine on one line and open it again on the next. So change stuff like this:

<?php session_start(); ob_start(); //used to change header without errors ?><?php if(isset($_SESSION['userid'])){ //Check if user is logged in ?><?php if(isset($_POST['cancel'])){ //If the cancel button is pressed in form, user redirected to acp page.	header("Location: acp.php");	exit();} ?><?php include('../includes/mf/header.php'); ?>

To this:

<?php session_start(); ob_start(); //used to change header without errorsif(isset($_SESSION['userid'])){ //Check if user is logged in    if(isset($_POST['cancel'])){ //If the cancel button is pressed in form, user redirected to acp page.	header("Location: acp.php");	exit();  }  include('../includes/mf/header.php'); ?>

Link to post
Share on other sites

When I make my own applications, I put things like that in functions. Every page in my application includes a configuration file, for example "global.conf.php". The configuration file typically looks something like this:

<?phpsession_start();global $SYSTEM_NAME;$SYSTEM_NAME = "My Application";global $HTTP_ADDRESS;$HTTP_ADDRESS = "http://www.server.com/appinstall/";...  // more variables definedrequire_once("functions.db.php");require_once("functions.auth.php");require_once("functions.session.php");require_once("functions.utility.php");require_once("messages.error.php");require_once("variables.status.php");?>

The point being that the one configuration file defines all of the necessary global variables, and includes whatever other files the application requires, such as for database connectivity, session handling, etc. In my functions.auth.php file, then I have a function written that will check the current session user against the database and redirects if the user is invalid, typically called auth_user. So, the top of each of my pages looks something like this:

<?phprequire_once("includes/global.conf.php");auth_user();...

And with those two lines, I can be assured that all necessary variables and functions have been defined, and the user is authenticated. If I make a new file, say variables.smilies.php or whatever, I can just include that page on the global.conf page and all pages have access to it. Having a single configuration file that all pages include makes it much easier to make quick global changes to your application.

Link to post
Share on other sites

I'm not really sure. The more you program, the more you realize how you keep doing certain things over and over (like connecting to a database, or authenticating the user, or whatever) and you will realize that it will save you time and effort if you create one function to do that and use the function wherever you need to (or, just automatically do it). Since nearly every page I do uses the database, my functions.db.php file not only defines the database username, password, etc, and a few functions to use the database (db_query, db_fetch_assoc, etc), but it also goes ahead and connects to the database server and selects the appropriate database (another variable). So including the configuration file, with the database file being included by it, gets me database connectivity, session management, everything that I would normally do to "set up" the page. After the first 3 or 4 lines on the page, I'm writing the code to do the actual processing that happens specifically on that page.

Link to post
Share on other sites

Yeah. I made all of my functions database-neutral (db_query instead of mysql_query) so that, in theory, I could change the db_query function if I wanted the application to use a different database. I've never had to do that, but it's a good idea, in theory.Another advantage is that my database functions have automatic error logging. I have an error logs table I set up in the database, and a typical call to db_query looks something like this:$result = db_query($sql, __FILE__, __LINE__);And then db_query executes the query using mysql_query, and if there is an error it inserts a new record in the logs table that records the error message, the query it tried to run, current user, file name, and line number. That way, if people tell me they are getting errors, I can log in and see all that information for myself.Here's my file, you can see for yourself. The write_to_log function is defined elsewhere and includes the current logged in user, and $nr_db_queries is initialized to 0 in global.conf, so that I can keep track of how many queries it takes to generate a page. Note the call at the end to db_connect.

<?php/******************************************************************************  functions.db.php  This contains database connectivity functions.******************************************************************************/################################################################################ database connectivity information:$DB_NAME		= "dbname";				 # name of the database$DB_SERVER	  = "localhost";			  # server address$DB_USER		= "sa";					 # server login$DB_PASSWORD	= "";					   # server password$DB_CONNECTED   = false;					# initialize to false, genius############################################################################################################################################################### db_connect#  this function connects to a database, with parameters given above###############################################################################function db_connect(){  global $DB_NAME, $DB_SERVER, $DB_USER, $DB_PASSWORD, $DB_CONNECTED;  if (!$DB_CONNECTED)  {	mysql_connect($DB_SERVER, $DB_USER, $DB_PASSWORD)	  or die("Could not connect to the database: <br>\n" . mysql_error());	mysql_select_db($DB_NAME)	  or die ("Could not select database $DB_NAME: <br>\n" . mysql_error());	$DB_CONNECTED = true;  }}############################################################################################################################################################### db_query#  this function sends a query to the database and returns the result###############################################################################function db_query($sql, $scriptName = "", $lineNumber = ""){  global $nr_db_queries;  if ($scriptName == "")	$scriptName = $_SERVER["SCRIPT_NAME"];  $scriptName = db_escape_string($scriptName);  $lineNumber = db_escape_string($lineNumber);  if (($result = @mysql_query($sql)) === false)  {	$db_error = mysql_error();	write_to_log("(<b> $scriptName </b> [$lineNumber])\nDATABASE ERROR: <br>\n" . $db_error . "<br>\nQuery:<br>\n{$sql}");	echo("(<b> $scriptName </b> [$lineNumber])\nDATABASE ERROR: <br>\nA database error has occured and has been saved in the log.");  }  $nr_db_queries++;  return $result;}############################################################################################################################################################### db_fetch_assoc#  wrapper to return an associative array###############################################################################function db_fetch_assoc($result){  return mysql_fetch_assoc($result);}############################################################################################################################################################### db_get#  wrapper to return an associative array from a query###############################################################################function db_get($sql, $file="", $line=""){  $file = ($file == "" ? __FILE__ : $file);  $line = ($line == "" ? __LINE__ : $line);  $res = db_query($sql, $file, $line);  if (db_num_rows($res) == 0)	return array();  else	return db_fetch_assoc($res);}############################################################################################################################################################### db_getvalue#  returns the specified field from the table based on the constraint###############################################################################function db_getvalue($table, $field, $constraint){  $row = db_get("SELECT $field FROM $table WHERE $constraint", $_SERVER['SCRIPT_NAME'], "db_getvalue");  return $row[$field];}############################################################################################################################################################### db_escape_string#  wrapper to return an escaped string###############################################################################function db_escape_string($str){  return mysql_real_escape_string($str);}function db_escstr($str) //alias{  return mysql_real_escape_string($str);}############################################################################################################################################################### db_num_rows#  wrapper to return the number of rows in a result###############################################################################function db_num_rows($result){  return mysql_num_rows($result);}############################################################################################################################################################### db_last_id#  wrapper to return the last id inserted###############################################################################function db_last_id(){  return mysql_insert_id();}############################################################################################################################################################### db_data_seek#  wrapper to seek in a result set###############################################################################function db_data_seek(&$result, $row){  mysql_data_seek($result, $row);}############################################################################################################################################################### db_exists#   returns true if the query results in a non-empty set, false otherwise###############################################################################function db_exists($sql, $file, $line){  if (db_num_rows(db_query($sql, $file, $line)) > 0)	return true;  else	return false;}###############################################################################db_connect();?>

Link to post
Share on other sites

Hmmmmm... This is very interesting. I understand most of it but could you provide an example of a page where you use one of those functions so I understand better.

Link to post
Share on other sites

Sure. Most of them just replace the built-in functions, some of them are for shortcuts.

<?phprequire_once("functions.db.php");$result = db_query("SELECT * FROM users WHERE id='" . db_escstr($_GET['id']) . "'", __FILE__, __LINE__);if ($row = db_fetch_assoc($result))  $id = $row['id'];//shortcut to get a row:$row = db_get("SELECT * FROM users WHERE id='" . db_escstr($_GET['id']) . "'", __FILE__, __LINE__);//get a single value:echo "The user's name is " . db_getvalue("users", "name", "id='" . db_escstr($id) . "'");if (db_exists("SELECT * FROM users WHERE id='admin'", __FILE__, __LINE__))  echo "admin exists";  $all_users = db_query("SELECT * FROM users", __FILE__, __LINE__);$user_1 = db_fetch_assoc($all_users);db_data_seek($all_users, 5);$user_5 = db_fetch_assoc($all_users);?>

Link to post
Share on other sites

On what? That was just a file I wrote, so what you see is what you get. This isn't really a specific style or "paradigm" or anything, so I doubt there would be formal documentation on how to program like this. It's just sort of.. how you program. Personal style.

Link to post
Share on other sites

Everything is going well. I only have 1 problem and that's the edit.php page. First of all on the edit page I first do a check to make sure the linkid is in the query string so I can perform my select then I select the record where the linkids are equal and fill the form for the user. Let's say the user erases the sitename completely (form field). The form will not validate and will refresh but then the linkid will not be in the query string anymore and will then be forwarded to the acp.php page before being able to change the link. I tried a bit with the session but should I even do that? What is the best way to have this form work well?btw when the user has updated his link he is redirected to acp.php and when the form does not validate he should stay on the form edit.php.

<?php //Start sessionsession_start(); ob_start(); if(isset($_SESSION['userid'])){ //Check if user is logged in	if(isset($_POST['cancel'])){ //If user clicks cancel		header("Location: acp.php"); //Redirect user to acp.php		exit();	} 	if(!isset($_REQUEST['linkid'])){ //If linkid is not in the query string		header("Location: acp.php");		exit();	}	if(!$_SESSION['linkid']){ //If linkid is not a session variable		$_SESSION['linkid'] = $_REQUEST['linkid']; //Set session variable linkid to query string linkid		$linkid = $_SESSION['linkid']; //Set session variable linkid to a $linkid	}	include('../includes/mf/header.php'); //Include header?>	<div class="fullcontent">		<h1>Edit Link</h1>		<?php 		if (isset($_POST['edit'])) { //Check if form is submitted			//Initialize error array			$errors = array();						//Validate sitename			if(empty($_POST['sitename'])){				$errors[] = "<p class=\"centerAlign\">Site name field is required.</p>";			} else {				$sitename = trim($_POST['sitename']);			}						//Validate siteurl			if(empty($_POST['siteurl'])){				$errors[] = "<p class=\"centerAlign\">Site URL field is required.</p>";			} else {				$siteurl = trim($_POST['siteurl']);			}								//If everything is OK			if(empty($errors)){				//Connect to database				require_once('../includes/mysql_connect.php');							//Query the database				$query2 = "UPDATE mf_links SET sitename = ".$_POST['sitename'].", siteurl = ".$_POST['siteurl']." WHERE linkid = ".$_SESSION['linkid'];								//Run query				$result2 = @mysql_query($query2);								//Erase session variable linkid				$_SESSION['linkid'] = NULL;							//If it ran OK				if($result2){					header("Location: acp.php");					exit();				} else { //If it didn't ran OK					echo "<h3>Your link was not submitted due to a system error.</h3>";					echo "<p>" . mysql_error() . "<br><br>Query: " . $query2 . "</p>";					include('../includes/mf/footer.php');					exit();				}				mysql_close();			} else {				//Display the errors				for($i=0 ; $i<sizeof($errors) ; $i++){					echo $errors[$i];				}			}		} 				//First query to fill text inputs in form		require_once('../includes/mysql_connect.php');		$query1 = "SELECT linkid, sitename, siteurl FROM mf_links WHERE linkid = ".$_SESSION['linkid'];				$result1 = @mysql_query($query1);				if($result1) {			while ($row = mysql_fetch_assoc($result1)){		?>		<form name="editlink" method="post" action="edit.php?linkid=<?php echo $linkid; ?>" class="form_addlink">			<div class="form_rowspacing"><label>Site Name</label> <input type="text" name="sitename" class="text_input" value="<?php echo $row['sitename']; ?>" /></div>			<div class="form_rowspacing"><label>Site URL</label>  http://<input type="text" name="siteurl"  class="text_input" value="<?php echo $row['siteurl']; ?>" /></div>			<hr />			<input type="submit" name="edit" value="Update" />			<input type="submit" name="cancel" value="Cancel" />		</form>		<?php			}			mysql_free_result($result1);		} else {			echo "<h3>A system error occured.</h3>";			echo "<p>" . mysql_error() . "<br><br>Query:" . $query1 . "</p>";			include('../includes/mf/footer.php');			exit();		}		mysql_close();		?>	</div><?php 	include("../includes/mf/footer.php");} else { //If user is not logged in	header("Location: index.php");	exit();} ?>

Link to post
Share on other sites

Yes I do. Right here: if(!$_SESSION['linkid']){ //If linkid is not a session variable$_SESSION['linkid'] = $_REQUEST['linkid']; //Set session variable linkid to query string linkid$linkid = $_SESSION['linkid']; //Set session variable linkid to a $linkid}

Link to post
Share on other sites

Ahh. But that only gets executed if it does not exist in the session. You may want to put this at the top of the page:$linkid = $_GET['linkid'];And then you can change that above code to this:if(!$_SESSION['linkid']){ //If linkid is not a session variable$_SESSION['linkid'] = $linkid; //Set session variable linkid to query string linkid}

Link to post
Share on other sites

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.   Paste as plain text instead

  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.

Loading...
×
×
  • Create New...