Hello,

is it possible to optimize/shorten this code snippet?

This code resides inside a while loop that runs atleast 100,000+ times, so i just wanted to know if it's possible.

// Workout if child category 1 exist in db
if ($child_cat_1_name != '' && strlen($child_cat_1_name) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_1` WHERE (`name` = '%s') LIMIT 1",
safeStr($child_cat_1_name)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
$child_cat_1_id = $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_1` SET `name` = '%s'",
safeStr($child_cat_1_name)));
$child_cat_1_id = mysql_insert_id();
}
}

// Workout if child category 2 exist in db
if ($child_cat_2_name != '' && strlen($child_cat_2_name) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_2` WHERE (`name` = '%s') LIMIT 1",
safeStr($child_cat_2_name)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
$child_cat_2_id = $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_2` SET `name` = '%s'",
safeStr($child_cat_2_name)));
$child_cat_2_id = mysql_insert_id();
}
}

// Workout if child category 3 exist in db
if ($child_cat_3_name != '' && strlen($child_cat_3_name) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_3` WHERE (`name` = '%s') LIMIT 1",
safeStr($child_cat_3_name)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
$child_cat_3_id = $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_3` SET `name` = '%s'",
safeStr($child_cat_3_name)));
$child_cat_3_id = mysql_insert_id();
}
}

// Workout if child category 4 exist in db
if ($child_cat_4_name != '' && strlen($child_cat_4_name) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_4` WHERE (`name` = '%s') LIMIT 1",
safeStr($child_cat_4_name)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
$child_cat_4_id = $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_4` SET `name` = '%s'",
safeStr($child_cat_4_name)));
$child_cat_4_id = mysql_insert_id();
}
}

// Workout if child category 5 exist in db
if ($child_cat_5_name != '' && strlen($child_cat_5_name) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_5` WHERE (`name` = '%s') LIMIT 1",
safeStr($child_cat_5_name)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
$child_cat_5_id = $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_5` SET `name` = '%s'",
safeStr($child_cat_5_name)));
$child_cat_5_id = mysql_insert_id();
}
}

Not sure how much faster this would be (or even if it work 100%) but:

<?php
for($i = 1; $i <5; $i++)
{
$var = 'child_cat_' . $i . '_name';
$cat_id = 'child_cat_' . $i . '_id';

if($$var != '' && strlen($$var) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_%d` WHERE (`name` = '%s') LIMIT 1",$i,safeStr($$var)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
$$cat_id = $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_%d` SET `name` = '%s'",$i,safeStr($$var)));
$$cat_id = mysql_insert_id();
}
}
}
?>

Since your 4 blocks are pretty much identical, I would suggest putting it inside a function.

function categoryExists($catNum, $catName) {
// Workout if child category 1 exist in db
if ($catName != '' && strlen($catName) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_catNum` WHERE (`name` = '%s') LIMIT 1",
safeStr($catName)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
return $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_catNum` SET `name` = '%s'",
safeStr($catName)));
return mysql_insert_id();
}
}

return 0;
}

$child_cat_1_id = categoryExists(1, $child_cat_1_name);
$child_cat_2_id = categoryExists(2, $child_cat_2_name);
$child_cat_3_id = categoryExists(3, $child_cat_3_name);
$child_cat_4_id = categoryExists(4, $child_cat_4_name);

You guys = too smart for me

Thanks allot, i like the function method, it reduces repetitive code.

There's a typo, this should be the right snippet:

function getChildCatID($catNum, $catName)
{
// Proceed if this is a valid $catName
if (($catNum >= 1 && $catNum <= 5) && ($catName != '' && strlen($catName) > 1))
{
// Check if $catName exists in the DB
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_`". $catNum ." WHERE (`name` = '%s') LIMIT 1",
safeStr($catName)));
if (mysql_num_rows($result) == 1)
{
// $catName does exists in the DB
$sql_row = mysql_fetch_array($result);
return $sql_row['id'];
}
else
{
// $catName doesn't exists in the DB
mysql_query(sprintf("INSERT INTO `child_cat_`". $catNum ." SET `name` = '%s'",
safeStr($catName)));
return mysql_insert_id();
}
}

return 0;
}

$child_cat_1_id = getChildCatID(1, $child_cat_1_name);
$child_cat_2_id = getChildCatID(2, $child_cat_2_name);
$child_cat_3_id = getChildCatID(3, $child_cat_3_name);
$child_cat_4_id = getChildCatID(4, $child_cat_4_name);
$child_cat_5_id = getChildCatID(5, $child_cat_5_name);

p.s. there there was 5 child categories, not 4 :P

Since your 4 blocks are pretty much identical, I would suggest putting it inside a function.

function categoryExists($catNum, $catName) {
// Workout if child category 1 exist in db
if ($catName != '' && strlen($catName) > 1)
{
$result = mysql_query(sprintf("SELECT `id` FROM `child_cat_catNum` WHERE (`name` = '%s') LIMIT 1",
safeStr($catName)));
if (mysql_num_rows($result) == 1)
{
$sql_row = mysql_fetch_array($result);
return $sql_row['id'];
}
else
{
mysql_query(sprintf("INSERT INTO `child_cat_catNum` SET `name` = '%s'",
safeStr($catName)));
return mysql_insert_id();
}
}

return 0;
}

$child_cat_1_id = categoryExists(1, $child_cat_1_name);
$child_cat_2_id = categoryExists(2, $child_cat_2_name);
$child_cat_3_id = categoryExists(3, $child_cat_3_name);
$child_cat_4_id = categoryExists(4, $child_cat_4_name);

Good approach just few comments on your and original code provided by topic starter:

1. It's always a bad taste to have function to do two things, in this case get data and then update data if no data is returned.

2. $catNum parameter is never used.

3. There is no need to check 'if ($catName != '' && strlen($catName) > 1)' since when using function passing a value for that parameter is required.

4. The actual INSERT statement is wrong, it looks like an UPDATE statement.

5. Since you are returning data when if (mysql_num_rows($result) == 1) is true, there is no need to have } else { bracket.


Here is my attempt:

<?php
/**
* Check if category exists
*
* @param string $categoryName category name to check
* @return mixed returns false on failure or category record id number
*/
function isCategory($categoryName)
{
$dbQuery = mysql_query(
sprintf("SELECT `id`
FROM `child_cat_catNum`
WHERE (`name` = '%s')
LIMIT 1", safeStr((string) $categoryName)));

if (mysql_num_rows($dbQuery) > 0) {
$dbResult = mysql_fetch_array($dbQuery);
return $dbResult['id'];
}

return false;
}

/**
* Insert new category
*
* @param string $categoryName category name to create
* @return bool returns false on failure or row id on success
*/
function insertCategory($categoryName)
{
$dbQuery = mysql_query(
sprintf("INSERT INTO `child_cat_catNum`
SET `name` = '%s'", safeStr((string) $categoryName)));

if(mysql_affected_rows() > 0) {
return mysql_insert_id();
}

return false;
}



$categoryId = isCategory($categoryName);
if (!$categoryId) {
$categoryId = insertCategory($categoryName);
}

Cheers!