Creating SQL queries for a MySQL database

Question

I am writing a PHP application that works with a MySQL database. As I am fairly new to PHP I am looking for ways to improve my code. Below are two functions, create_ct_query and create_ct_query2. create_ct_query2 is the result of my refactoring create_ct_query. I am fairly happy with it now, but am wondering how it looks to others.

<?php
// experiment with versions of create_ct_query

$lvb_id_ml=30;       // lvb id
$lvb_name_ml=100;    // competition name

$lvb_competitions_table =
  array ("table_name" => "lvb_competitions",
         "table_cols" => 
         array (array ('id', $lvb_id_ml),
                array ('name', $lvb_name_ml),
                array ('age', 'INT')),
         "primary_key" => array ('id', 'age'));


function varchar ($len) {
  return "VARCHAR(" . $len . ")";
}

function primary_key ($item) {
  return $item . " PRIMARY KEY";
}


// version currently in script
function create_ct_query ($tdesc) {
  $ret_val = "CREATE TABLE " . $tdesc['table_name'] . " ( ";

  foreach ($tdesc["table_cols"] as $row) {
    if (is_numeric ($row[1])) {
      $ret_val .= $row[0] . " " . varchar ($row[1]) . ", ";
    } else {
      $ret_val .= $row[0] . " " . $row[1] . ", ";
    }
  }

  // add primary key constraint
  $ret_val .= " CONSTRAINT pk_" . $tdesc["table_name"] .
    " PRIMARY KEY (";
  if (is_array ($tdesc['primary_key'])) {
    $ret_val .= implode (",", $tdesc['primary_key']);
  } elseif (is_string ($tdesc['primary_key'])) {
    $ret_val .= $tdesc["primary_key"];
  } else {
    die ("Primary key not of right type in tdesc for " .
         $tdesc['table_name'] . "\n");
  }
  $ret_val .= "))";

  return $ret_val;
}

function create_ct_query2 ($tdesc) {
  $ret_val = "CREATE TABLE " . $tdesc['table_name'] . " (";

  $ret_val .= cols_desc ($tdesc['table_cols']) . ", ";

  $ret_val .= primary_key_constraint ($tdesc['table_name'],
                                      $tdesc['primary_key']) . "))";

  return $ret_val;
}


// create the item description from the item
// Example:
// item_desc (array ('name', 10));
//    'name VARCHAR(10)'
// item_desc (array ('name', "INT"));
//    'name INT'
function item_desc ($item) {
  return " " . $item[0] . " " .
    (is_numeric($item[1]) ? varchar($item[1]) : $item[1]);
}

// create the column description part of table creation query
// Example:
// cols_desc (array (array ('id', 10), array ('id2', INT)));
//   'id VARCHAR(10), id2 INT'
function cols_desc ($table_cols) {
  return implode (",", array_map ('item_desc', $table_cols));
}


// create the primary key constraint part of the table creation query
// primary_key_constraint ('name', 'id');
//    CONSTRIANT pk_name PRIMARY KEY (id)
// primary_key_constraint ('name', array ('id', 'id2'));
//    CONSTRAINT pk_name PRIMARY_KEY (id, id2)
function primary_key_constraint ($table_name, $primary_key_desc)  {
  return " CONSTRAINT pk_" . $table_name . " PRIMARY KEY (" . 
    (is_array($primary_key_desc) ? 
     implode (",", $primary_key_desc) :
     $primary_key_desc);
}


echo create_ct_query ($lvb_competitions_table) . "\n\n";
echo create_ct_query2 ($lvb_competitions_table) . "\n\n";

Answer 1

Assuming you're protecting yourself from SQL Injection and the like, there are just a couple things that stand out to me:

• It doesn't seem clear to me why the closing parenthesis for your primary key constraint is in the create_ct_query2() function rather than in the primary_key_constraint() function.
• I believe the code would be far easier to read, test, and extend if it were built using objects rather than global functions.
• Since you are new to PHP, I think it is worth mentioning that single quotes and double quotes are treated differently. Knowing this, as a personal preference, I tend to use single quotes unless I intentionally need / want to use the added functionality of double quotes.
• While related to refactoring, I believe my other suggestions start to delve more in to the readability of things like create_ct_query(). This starts to feel a bit nit-picky, and almost a matter of style. For example, with create_ct_query(), it took me a minute to realize what "ct" meant. Only after looking inside the function and seeing the string being created did I realize what it did. I feel that something like "get_create_table_query()" or similar would be a bit more intuitive.

Regarding documentation, I have found Eclipse and other tools to utilize the following style of doc-blocks. The @return is especially useful for code-completion of methods available on the returning object in my IDE. Documentation tools tend to have built-in support for other tags like @author as well.

/**
 * Description of someMethod()
 *
 * @param int $someInt
 * @param ObjectName $someObj
 * @return AnotherObj
 */
public function someMethod($someInt, ObjectName $someObj)
{
    // ... 

    return new AnotherObj();
}

Answer 2

Stylistically, it looks okay to me. But technically, you will want to embrace all your table and column names in backtick quotes, or else your queries will often fail if they are coming from user input. E.g. table and column names need backticks if they contain dashes, and also for a lot of other characters. See description here.

On a second note, of course, you will want make sure that those variables that contain table and field names are properly filtered.