More Efficient Variable Check For Relevant Query

[son]

Having a form to upload product data I came over the problem that I allow user to upload one to three photos. One is obligatory, the rest is up to them. To update image names with product id I run the relevant select query, but my checks for img1, img2 and so are make the whole thing rather long. Is there a way to make this more efficient? My code so far is:

if (isset($_POST['submitted']))	{//initialise error array  $errors = array();        if (isset($_POST['category']) && ($_POST['category']) != 0)	{		$category = escape_data($_POST['category']);	}	else	{		$category = FALSE;		$errors['category'] = '\'Category\' is required';		}        if (isset($_POST['designer']) && ($_POST['designer']) != 0)	{		$designer = escape_data($_POST['designer']);	}	else	{		$designer = FALSE;		$errors['designer'] = '\'Designer\' is required';		}        if (isset($_POST['season']) && ($_POST['season']) != 0)	{		$season = escape_data($_POST['season']);	}	else	{		$season = FALSE;		$errors['season'] = '\'Season\' is required';		}	if (!isset($_POST['product']) OR empty($_POST['product'])) {	$product = FALSE;	$errors['product'] = '\'Product\' is required';		} else	{		$product = escape_data($_POST['product']);	}	if (!isset($_POST['short_desc']) OR empty($_POST['short_desc'])) {	$short_desc = FALSE;	} else	{		$short_desc = escape_data($_POST['short_desc']);	}	if (!isset($_POST['price']) OR empty($_POST['price'])) {	$price = FALSE;	$errors['price'] = '\'Price\' is required';		} else	{		$price = escape_data($_POST['price']);	}	if (!isset($_POST['description']) OR empty($_POST['description'])) {	$description = FALSE;	$errors['description'] = '\'Description\' is required';		}	else	{	$description = escape_data($_POST['description']);	}  // check for img1  if (!isset($_FILES['img1']['name']) OR empty($_FILES['img1']['name']) OR $_FILES['img1']['error'] == 4) {      $errors['img1'] = '\'Image 1\' is a required field';    $img1 = FALSE;  } else {    if ($_FILES['img1']['error'] != 0)    {      $errors['img1'] = upload_error($_FILES['img1']['error']);      $img1 = false;    }    else    {      $ext = explode('.',$_FILES['img1']['name']);      $ext = $ext[count($ext)-1];      if (!in_array(strtolower($ext), $allowed)) 	  {        $errors['img1'] = 'Allowed formats: jpg, gif, png';        $img1 = FALSE;      } 	  else 	  {        $img1 = "1.{$ext}";		$img1 = strtolower($img1);        if (!move_uploaded_file($_FILES['img1']['tmp_name'], "site/products/main/{$img1}"))          $errors['img1'] = 'The file could not be moved';        else        {          resize_image("{$main_img}/{$img1}", "{$main_img}/{$img1}", IMG_SIZE_W, IMG_SIZE_H);          resize_image("{$main_img}/{$img1}", "{$preview_img}/{$img1}", THUMB_SIZE_W, THUMB_SIZE_H);        }      }    }  }  // check for img2  if (!isset($_FILES['img2']['name']) OR empty($_FILES['img2']['name']) OR $_FILES['img2']['error'] == 4) {    $img2 = FALSE;  } else {    if ($_FILES['img2']['error'] != 0)    {      $errors['img2'] = upload_error($_FILES['img2']['error']);      $img2 = false;    }    else    {      $ext = explode('.',$_FILES['img2']['name']);      $ext = $ext[count($ext)-1];      if (!in_array(strtolower($ext), $allowed)) 	  {        $errors['img2'] = 'Allowed formats: jpg, gif, png';        $img2 = FALSE;      } 	  else 	  {        $img2 = "2.{$ext}";		$img2 = strtolower($img2);        if (!move_uploaded_file($_FILES['img2']['tmp_name'], "site/products/main/{$img2}"))          $errors['img2'] = 'The file could not be moved';        else        {          resize_image("{$main_img}/{$img2}", "{$main_img}/{$img2}", IMG_SIZE_W, IMG_SIZE_H);          resize_image("{$main_img}/{$img2}", "{$preview_img}/{$img2}", THUMB_SIZE_W, THUMB_SIZE_H);        }      }    }  }  // check for img3  if (!isset($_FILES['img3']['name']) OR empty($_FILES['img3']['name']) OR $_FILES['img3']['error'] == 4)   {    $img3 = FALSE;  }   else   {    if ($_FILES['img3']['error'] != 0)    {      $errors['img3'] = upload_error($_FILES['img3']['error']);      $img3 = false;    }    else    {      $ext = explode('.',$_FILES['img3']['name']);      $ext = $ext[count($ext)-1];      if (!in_array(strtolower($ext), $allowed)) 	  {        $errors['img3'] = 'Allowed formats: jpg, gif, png';        $img3 = FALSE;      } 	  else 	  {        $img3 = "3.{$ext}";		$img3 = strtolower($img3);        if (!move_uploaded_file($_FILES['img3']['tmp_name'], "site/products/main/{$img3}"))          $errors['img3'] = 'The file could not be moved';        else        {          resize_image("{$main_img}/{$img3}", "{$main_img}/{$img3}", IMG_SIZE_W, IMG_SIZE_H);          resize_image("{$main_img}/{$img3}", "{$preview_img}/{$img3}", THUMB_SIZE_W, THUMB_SIZE_H);        }      }    }  }	if ($product  && $description && $price && $img1 && $category && $designer && $season)		{	$query2 = "INSERT INTO products (product, short_desc, description, price, designer_id, season_id) VALUES ('$product', '$short_desc', '$description', '$price', '$designer', '$season')";		if ($result2 = mysqli_query ($dbc, $query2))			{							$id = mysqli_insert_id($dbc);   			if ($id > 0) 			{			if ($img1 && $img2 && $img3)			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}', img2 = '{$id}-{$img2}', img3 = '{$id}-{$img3}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}			elseif ($img1 && $img2)			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}', img2 = '{$id}-{$img2}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}				elseif ($img1 && $img3)			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}', img3 = '{$id}-{$img3}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}				else			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}					$productAssocQ = "INSERT INTO productCat (category_id, product_id) VALUES ('$category', '$id')";				{				if ($productAssocRes = mysqli_query ($dbc, $productAssocQ))					echo "<p>The product has been added!<br />";				echo "<span class=\"small\"><a href=\"products.php\"><< Go back</a></span></p>";				include('inc/footer.php');				exit;				}	 		}		}			else

Son

[justsomeguy]

You need to identify the parts of the code that change (the image names, IDs, etc), and build a loop to use variables for those instead.

$images = array(  'img1' => 'Image 1',  'img2' => 'Image 2',  'img3' => 'Image 3');foreach ($images as $img_id => $img_name){  if (!isset($_FILES[$img_id]['name']) OR empty($_FILES[$img_id]['name']) OR $_FILES[$img_id]['error'] == 4) {	$errors[$img_id] = '\'' . $img_name . '\' is a required field';	$$img_id = FALSE;  } else {	if ($_FILES[$img_id]['error'] != 0)	{	  $errors[$img_id] = upload_error($_FILES[$img_id]['error']);	  $$img_id = false;	}	else	{	  $ext = explode('.',$_FILES[$img_id]['name']);	  $ext = $ext[count($ext)-1];	  if (!in_array(strtolower($ext), $allowed))	  {		$errors[$img_id] = 'Allowed formats: jpg, gif, png';		$$img_id = FALSE;	  }	  else	  {		$img_num = trim('Image', '', $img_name);		$$img_id = "{$img_num}.{$ext}";		$$img_id = strtolower($$img_id);		if (!move_uploaded_file($_FILES[$img_id]['tmp_name'], "site/products/main/{$$img_id}"))		  $errors[$img_id] = 'The file could not be moved';		else		{		  resize_image("{$main_img}/{$$img_id}", "{$main_img}/{$$img_id}", IMG_SIZE_W, IMG_SIZE_H);		  resize_image("{$main_img}/{$$img_id}", "{$preview_img}/{$$img_id}", THUMB_SIZE_W, THUMB_SIZE_H);		}	  }	}  }}

[son]

You need to identify the parts of the code that change (the image names, IDs, etc), and build a loop to use variables for those instead.

  [_FILES] => Array        (            [img1] => Array                (                    [name] => png_upper.PNG                    [type] => image/png                    [tmp_name] => /tmp/phpmnVyk9                    [error] => 0                    [size] => 3057                )            [img2] => Array                (                    [name] => jpg_upper.JPG                    [type] => image/jpeg                    [tmp_name] => /tmp/php9kBvIP                    [error] => 0                    [size] => 10774                )            [img3] => Array                (                    [name] => gif_lower.gif                    [type] => image/gif                    [tmp_name] => /tmp/phpgiAZow                    [error] => 0                    [size] => 1353                )        )

Why would that not work?Son

[justsomeguy]

Left that out somehow, should have been this:$img_num = trim(str_replace('Image', '', $img_name));

[son]

Left that out somehow, should have been this:$img_num = trim(str_replace('Image', '', $img_name));

That is great! Much shorter code... Son

[son]

That is great! Much shorter code... Son

Is there actually also a way to shorten the query bit below by building up the query in the loop? The relevant code is:

if ($img1 && $img2 && $img3)			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}', img2 = '{$id}-{$img2}', img3 = '{$id}-{$img3}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}			elseif ($img1 && $img2)			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}', img2 = '{$id}-{$img2}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}				elseif ($img1 && $img3)			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}', img3 = '{$id}-{$img3}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}				else			{     		$upd_query = "UPDATE products SET img1 = '{$id}-{$img1}' WHERE product_id = '$id' ";			$upd_result = mysqli_query ($dbc, $upd_query);			}		

That would be great. I tend to write long scripts and welcome to learn all about shortening my code (which will make maintenance easier).Son

[justsomeguy]

$upd = "";$sql = "UPDATE products SET";if ($img1)  $upd .= ($upd != '' ? ',' : '') . " img1 = '{$id}-{$img1}'";if ($img2)  $upd .= ($upd != '' ? ',' : '') . " img2 = '{$id}-{$img2}'";if ($img3)  $upd .= ($upd != '' ? ',' : '') . " img3 = '{$id}-{$img3}'";$sql .= $upd . " WHERE product_id = '$id' ";if ($upd != '')  $upd_result = mysqli_query ($dbc, $sql);

[son]

foreach ($images as $img_id => $img_name){  if (!isset($_FILES[$img_id]['name']) OR empty($_FILES[$img_id]['name']) OR $_FILES[$img_id]['error'] == 4)   {  		if ($img_id == 'img1')		{    	$errors[$img_id] = '\'' . $img_name . '\' is a required field';		}    $$img_id = FALSE;  }   else   {    if ($_FILES[$img_id]['error'] != 0)    {      $errors[$img_id] = upload_error($_FILES[$img_id]['error']);      $$img_id = false;    }    else    {      $ext = explode('.',$_FILES[$img_id]['name']);      $ext = $ext[count($ext)-1];      if (!in_array(strtolower($ext), $allowed))      {        $errors[$img_id] = 'Allowed formats: jpg, gif, png';        $$img_id = FALSE;      }      else      {        $img_num = trim(str_replace('Image', '', $img_name));         $$img_id = "{$img_num}.{$ext}";        $$img_id = strtolower($$img_id);        if (!move_uploaded_file($_FILES[$img_id]['tmp_name'], "site/products/main/{$$img_id}"))          $errors[$img_id] = 'The file could not be moved';        else        {          resize_image("{$main_img}/{$$img_id}", "{$main_img}/{$$img_id}", IMG_SIZE_W, IMG_SIZE_H);          resize_image("{$main_img}/{$$img_id}", "{$preview_img}/{$$img_id}", THUMB_SIZE_W, THUMB_SIZE_H);        }      }    }  }}	if ($product  && $description && $price && $img1 && $category && $designer && $season)		{	$query2 = "INSERT INTO products (product, short_desc, description, price, category_id, designer_id, season_id) VALUES ('$product', '$short_desc', '$description', '$price', '$category', '$designer', '$season')";		if ($result2 = mysqli_query ($dbc, $query2))			{							$id = mysqli_insert_id($dbc);   			if ($id > 0) 			{			$upd = "";			$sql = "UPDATE products SET";			if ($img1)  			$upd .= ($upd != '' ? ',' : '') . " img1 = '{$id}-{$img1}'";			if ($img2)  			$upd .= ($upd != '' ? ',' : '') . " img2 = '{$id}-{$img2}'";			if ($img3)  			$upd .= ($upd != '' ? ',' : '') . " img3 = '{$id}-{$img3}'";			$sql .= $upd . " WHERE product_id = '$id' ";			if ($upd != '')  			$upd_result = mysqli_query ($dbc, $sql);

I tried to put:

 if (!move_uploaded_file($_FILES[$img_id]['tmp_name'], "site/products/main/{$$img_id}"))          $errors[$img_id] = 'The file could not be moved';        else        {          resize_image("{$main_img}/{$$img_id}", "{$main_img}/{$$img_id}", IMG_SIZE_W, IMG_SIZE_H);          resize_image("{$main_img}/{$$img_id}", "{$preview_img}/{$$img_id}", THUMB_SIZE_W, THUMB_SIZE_H);        }

under the code that gets $id, but then it does not work at all (the file naming on server). How can I move this bit under with it working?Son

[justsomeguy]

You should probably insert a new record first and get the ID. As you're going through the validation, if anything fails and creates an error just delete the new record from the database.

[son]

You should probably insert a new record first and get the ID. As you're going through the validation, if anything fails and creates an error just delete the new record from the database.

Have done this and seems to work fine. I have one more question with regard to files on server. I never worked on scripts that deleted files. Now I know how to delete data from database. My code is:

// delete a product after confirmation	if (isset($_GET['conf']) && $_GET['conf'] == 'yes')		{	$del_q = "DELETE FROM products WHERE product_id = $id";		if ($del_r = mysqli_query ($dbc, $del_q))			{		echo "<p>The product has been deleted!<br />";		echo "<span class=\"small\"><a href=\"products.php\"><< Go back</a></span></p>";		include('inc/footer.php');		exit;		}	}

Can I at the same time delete the relevant image files from server? Might overcrowd my server otherwise... The naming is corresponding to insert queries ({$main_img}/{$id}-{$$img_id} and {$preview_img}/{$id}-{$$img_id} with images running from 1 to 3). Only the first one is definitely on server for any existing id.Son

[justsomeguy]

Select the image names before deleting the record, for each image name check if it's not empty and, if not, you can use the file_exists function to check that the file actually exists, and then the unlink function to delete it. For additional validation you can throw in a call to is_dir to make sure it's a file name and not a directory name you're about to delete.

[son]

Select the image names before deleting the record, for each image name check if it's not empty and, if not, you can use the file_exists function to check that the file actually exists, and then the unlink function to delete it. For additional validation you can throw in a call to is_dir to make sure it's a file name and not a directory name you're about to delete.

Will have a go...Many thanks,Son