[RESOLVED] Organizing MySQL queries

[gwerner]

I've been in the process of going back through an existing site I've developed. I'm trying to better organize and streamline the code I initially put together. I've been pulling apart my code and building towards a more modular approach.

Originally, I had included all of my queries inline along with my html. Upon further reading it seems like this might not be the best practice. I'm looking for tips and best practices when dealing with organization of mySQL queries.

My first thoughts would be to put the most common queries inside functions and access them through a require_once. It seems weird though to call a function grabData() within the html. Am I thinking about this correctly?

Most of my pages only need one to two queries, while others might have up to seven plus. The higher numbered queries would be on a dashboard pages where a lot of data is being displayed.

Another example would be a states database that is accessed quite often through various form pages. It seems like I should build the query once and then call from a function?

Any insight or guidance is appreciated. Thanks.

[ixalmida]

I think the gurus will tell you that your best bet is to build functions or classes that will do all the validation and scrubbing. Then you only need to pass fields, values, and possibly extra conditions to the function/class.

I'd even raise you one on the states thing. I have built functions to retrieve states and display them in a drop-down box. I use the stateSelectBox() function in many of my forms.

However...encapsulating queries this way may not always be more efficient, and in many cases it obscures your code from yourself. I tend to like to do things as simply as possible. If that means building a function or class for queries that get used frequently, then I'll do that. But sometimes it is a waste of my time because the query is unique to the page. IMO you should consider it on a case-by-case basis.

I've actually seen people build functions/classes for all queries in general, but in the end they have to pass almost the entire query to the function and reassemble it on the other side. Where's the efficiency in that?

[NogDog]

For the state select box example, it might be two separate functions: one to retrieve the states from the DB as an array, and a generic function to display a select element from an array.

db_functions.php:

PHP Code:

<?php

/**
* Get states from DB as array
* @return array
*/
function getStateArray()
{
   $sql = "SELECT `abbrev`, `name` FROM `state` ORDER BY `name`";
   $result = mysql_query($sql);
   if(!$result)
   {
      error_log(mysql_error()."\n$sql");
      return false;
   }
   $arr = array();
   while($row = mysql_fetch_assoc($result))
   {
      $arr[$row['abbrev']] = $row['name'];
   }
   return $arr;
}

html_functions.php:

PHP Code:

<?php
/**
* get <select> HTML for an array of data
* @return string
* @param string $name Select element name & ID
* @param array $data  Key: option value, Value: displayed value
* @param string $class Select element class (optional)
*/
function selectFromArray($name, $data, $class=null)
{
   $html = sprintf(
      "<select name='%s' id='%s'%s>\n",
      htmlspecialchars($name, ENT_QUOTES),
      htmlspecialchars($name, ENT_QUOTES),
      (empty($class)) ? '' : " class='".htmlspecialchars($class,ENT_QUOTES)."'"
   );
   foreach($data as $key => $value)
   {
      $html .= sprintf(
         "<option value='%s'>%s</option>\n",
         htmlspecialchars($key, ENT_QUOTES),
         htmlspecialchars($value, ENT_QUOTES)
      );
      $html .= "</select>\n";
   }
   return $html;
}

index.php:

PHP Code:

<?php
require_once 'db_functions.php';
require_once 'html_functions.php';
?>
<form action='' method='post'>
<fieldset>
<legend>Test</legend>
<label for='state'>Select state:</label>
<?php echo selectFromArray('state', getStateArray(), 'class_foo'); ?>
</fieldset>
</form>

If you are reasonably comfortable with the basics of object-oriented PHP, the next step might be to organize related functions into classes.

[ixalmida]

Nog --> great post!

[gwerner]

Thanks for the help. I'm doing something similar to what NogDog posted. My code on index.php is not nearly as clean though. Have to go back and retool it a bit.

This helps out quite a bit. Still learning and trying to wrap my head around all of this. Thanks guys.

[gwerner]

Actually another quick question. After looking through NogDog's suggestion I went back and retooled my code a bit now have a cleaner bit of code.

I'm using this in a form and I like to do error checking to help the user if they forget a field or input incorrect data. For dropdowns I check to see if the field is set to -1, which is the default selction of "Choose below..." Anything else should be a state selection. For NogDog's example I would normally add the code below. Is this the right approach?

PHP Code:

<?php
if (isset($_POST['state'])) {
    $selected = $_POST['state'];
}
else {
    $selected = '-1';
}
echo dropdown('state', getStates(), $selected);
?>

[NogDog]

Originally Posted by gwerner

Actually another quick question. After looking through NogDog's suggestion I went back and retooled my code a bit now have a cleaner bit of code.

I'm using this in a form and I like to do error checking to help the user if they forget a field or input incorrect data. For dropdowns I check to see if the field is set to -1, which is the default selction of "Choose below..." Anything else should be a state selection. For NogDog's example I would normally add the code below. Is this the right approach?

PHP Code:

<?php
if (isset($_POST['state'])) {
    $selected = $_POST['state'];
}
else {
    $selected = '-1';
}
echo dropdown('state', getStates(), $selected);
?>

There is nothing wrong with that, but if you're one of those people who likes terse code, you could do:

PHP Code:

<?php
echo dropdown('state', getStates(), isset($_POST['state']) ? $_POST['state'] : -1);
?>

Functionally it gains you nothing. In readability it probably loses something. In performance it might save a microsecond or two, typing-wise it saves a few seconds.

[gwerner]

I do like me some terse code. I've been noticing this style more and more and would like to learn how to write in this manner. Do you have a good resource on how to write code this way?

Thanks again for all the help.

[bradgrafelman]

Originally Posted by gwerner

Do you have a good resource on how to write code this way?

Not sure what you mean by "this way." If you mean taking an if/else statement and shortening it to one line, then you're looking at using the ternary operator.

[gwerner]

Yes, that is what I meant. Sorry. I'm just not familiar with how that looks compared to the traditional if/else. And, I didn't know if this was the only instance you can write the code in this manner or if there were others as well. You answered my question, and thanks for the link. I keep learning new stuff every day.