Infernate while loop

knowj

For some reason this is just looping infernally. I'm sure its something really stupid but it's getting the better of me today.

SQL query/method

public function getimages()
		{
			if ($results = $this->database->query("SELECT * FROM `products` WHERE processed!='1'"))
				{
					return $results->fetch_array(MYSQLI_ASSOC);
				}
		}

process_image.php

<?php
$products = products::getinstance();
$url = "/images/products/";
//set $row with the results from get images


while ($row = $products->getimages())
	{
		$temp = tempnam("tmp/", "IMG_");
		$handle = fopen($temp, "w+");
		fwrite($handle, $row['front_img']);
		$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
		$thumb = $url.$image->create_image('thumb');

	$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
	$img = $url.$image->create_image('normal');

	$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
	$detail = $url.$image->create_image('detail');

	if (!$products->saveimages($row['id'], $thumb, $img, $detail, 'front'))
		{
			echo 'fail - id: '.$row['id'].'- front';
		}
	fclose($handle); // this removes the file
	unset($thumb, $img, $detail);


	//if there is a back image process it
	if ($row['back_img'])
		{
			$temp = tempnam("tmp/", "IMG_");
			$handle = fopen($temp, "w+");
			fwrite($handle, $row['back_img']);
			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$thumb = $url.$image->create_image('thumb');

			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$img = $url.$image->create_image('normal');

			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$detail = $url.$image->create_image('detail');

			if (!$products->saveimages($row['id'], $thumb, $img, $detail, 'back'))
				{
					echo 'fail - id: '.$row['id'].'- back';;
				}
			fclose($handle); // this removes the file
			unset($thumb, $img, $detail);
		}


	//if there is a detail 1 image process it
	if ($row['detail_img1'])
		{	
			$temp = tempnam("tmp/", "IMG_");
			$handle = fopen($temp, "w+");
			fwrite($handle, $row['detail_img1']);
			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$thumb = $url.$image->create_image('thumb');

			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$img = $url.$image->create_image('normal');

			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$detail = $url.$image->create_image('detail');

			if (!$products->saveimages($row['id'], $thumb, $img, $detail, 'detail1'))
				{
					echo 'fail - id: '.$row['id'].'- detail';
				}
			fclose($handle); // this removes the file
			unset($thumb, $img, $detail);
		}

	//if there is a detail 2 image process it
	if ($row['detail_img2'])
		{	
			$temp = tempnam("tmp/", "IMG_");
			$handle = fopen($temp, "w+");
			fwrite($handle, $row['detail_img2']);
			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$thumb = $url.$image->create_image('thumb');

			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$img = $url.$image->create_image('normal');

			$image = new image($temp, constants::UPLOADDIR_PRODUCTS);
			$detail = $url.$image->create_image('detail');

			if (!$products->saveimages($row['id'], $thumb, $img, $detail, 'detail2'))
				{
					echo 'fail - id: '.$row['id'].'- detail 2';
				}
			fclose($handle); // this removes the file
			unset($thumb, $img, $detail);
		}
}
?>

Note: There is only 1 row in the database

laserlight

I am guessing that by "infernally" you mean "infinitely", though I agree that uncontrolled infinite loops can be hell 🙂

The problem is this: getimages() executes the query. So, everytime you call getimages(), it executes the query, retrieves the first result, and returns it. Now, you run getimages() in a loop. So on the first iteration, the first row of the result set is retrieved, and then returned. Then on the second iteration, the first row of the result set is retrieved, and then return. So on and so forth, unless in your loop body you truncate (remove all the rows of) the table, upon which the next call of getimages() would have nothing to select, and finally returns false to end your loop.

knowj

So I basically need to separate the query from the return?

also how would you go about it to try to keep the code tidy?

cheers
J

laserlight

A simple and clean way is to convert the result set into an array and then return that array. However, if there are many rows in the table, this approach could be impractical.

Another approach is to use the iterator design pattern. getimages() would return an image iterator object that stores the mysqli_result object returned by the query. So, to get the next image, the caller would invoke a next() member function of the image iterator object.

knowj

The iterator pattern seems pretty excessive looking at it.

laserlight

The iterator pattern seems pretty excessive looking at it.

It is more work, but the advantage is that you do not have to load the whole result set into memory.

knowj

Is the iterator pattern the most used method within PHP/MySQL apps then?

Or have I just messed up at an early point and now have to use it?

NogDog

Could you not just do...

        public function getimages()
        {
            if ($results = $this->database->query("SELECT * FROM `products` WHERE processed!='1'"))
            {
                return $results;
            }
            else
            {
                // log error
                return false;
            }
        }

$result = $products->getimages();
while ($row = $result->fetch_assoc())
{

...?

laserlight

knowj wrote:
Is the iterator pattern the most used method within PHP/MySQL apps then?

I am not sure about "most used", sorry, but it is generally quite commonly used.

knowj wrote:
Or have I just messed up at an early point and now have to use it?

It looks like you are still in the design stage for this part, so there certainly is no problem.

NogDog wrote:
Could you not just do...

You could, but then you lose some of the benefit of the encapsulation since the caller would also be tied to the same database API. Other than that it would be an example of the iterator pattern, since a mysqli_result is an iterator even though it does not implement the SPL's iterator interface.