[RESOLVED] Trouble with nested foreach()

coffejor

I have a set of nested foreach() functions that I use to organize products by status and category. When I run the code (Example) it only goes through the first category. I had put an echo after the first foreach() to ensure that it is truly looping through the array and it resulted in the proper SQL and number of SQL statements.

Here is my code (** For the sake of brevity, I have removed the code used to format the content)

foreach ($status_array as $status=>$text) {
    // Request all the information for the products with the right status
    $sql = "SELECT * FROM products WHERE status = '$status'";
    $result = @mysql_query($sql);
    $num = mysql_num_rows($result);

if ($num > 0) {

    $content .= <<<EOF
    <h2>$text Products</h2>
EOF;
        foreach ($categories as $category => $info) {
            $sql = "SELECT * FROM products WHERE status = '$status' AND category = '$category'";
            $result = @mysql_query($sql);
            $num = mysql_num_rows($result);

        if ($num > 0) {

            // Display the text of each product in table row
            while ($row = mysql_fetch_array($result, MYSQL_ASSOC)) {
                $id = $row["id"];
                $image = $row["image"];
                $category = $row["category"];
                $sdescription = $row["sdescription"];
                $description = $row["description"];
                $price = $row["price"];
                $status = $row["status"];
                // Alternate the row colors
                $alternate = ($alternate == 'alternate' ? 'normal' : 'alternate');
                // If an image exists, get its dimensions
                if ($image2 = @getimagesize (URL."/products/thumbnail/small_{$row['image']}")) {
                    $image_size = getimagesize (URL."/products/{$row['image']}");
                } else {
                    $image_size = 0;
                }

                $content .= <<<EOF
                    ** SCRIPTING TO DISPLAY CONTENT WENT HERE **
EOF;
                }
            }
        }
    }
}

Any and all advice / insight is appreciated.

~Jordan

laserlight

At a glance, I am not sure what is the problem. The reuse of $result looks dangerous, but I reason that it is not a problem in this case (other people have had problems where they reused the variable that stored the query result, retrieved the results in a loop using this variable, and then assigned to that variable in the loop).

However, looking more closely at your code logic, it seems that the outer query is performed solely to determine if the given status had any corresponding entries in the products table. This suggests an optimisation that eliminates one of the nested loops, e.g.,

$category_list = "'" . implode("','", array_keys($categories)) . "'";
foreach ($status_array as $status => $text) {
    $sql = "SELECT * FROM products WHERE status = '$status' AND category IN ($category_list)";
    $result = mysql_query($sql);

if (mysql_num_rows($result) > 0) {
    $content .= <<<EOF
    <h2>$text Products</h2>
EOF;

    while ($row = mysql_fetch_array($result, MYSQL_ASSOC)) {
        $id = $row["id"];
        $image = $row["image"];
        $category = $row["category"];
        $sdescription = $row["sdescription"];
        $description = $row["description"];
        $price = $row["price"];
        $status = $row["status"];
        // Alternate the row colors
        $alternate = ($alternate == 'alternate' ? 'normal' : 'alternate');
        // If an image exists, get its dimensions
        if ($image2 = @getimagesize (URL."/products/thumbnail/small_{$row['image']}")) {
            $image_size = getimagesize (URL."/products/{$row['image']}");
        } else {
            $image_size = 0;
        }

        $content .= <<<EOF
            ** SCRIPTING TO DISPLAY CONTENT WENT HERE **
EOF;
        }
    }
}

coffejor

Laserlight,

Thanks for the suggestion. The goal of the script was to go through each of three statuses: Active, Inactive, and On Sale. Within each of the status I wanted to return all the products that match that status and have them grouped by their category.

For example: If I had 10 active products belonging to the Apparel category, 5 active products belonging to the Books category, 3 inactive products belonging to the Apparel category, 1 active product belonging to the Books category, 1 On Sale product belonging to the Apparel Category, and 5 On Sale Products belonging to the Books category, it would subsequently display the items as follows,

-- Active Products--

[INDENT] Apparel[/INDENT]
[INDENT][INDENT] Product 1
Product 2
.
.
.[/INDENT][/INDENT]

[INDENT] Books[/INDENT]
[INDENT][INDENT] Product 1
Product 2
.
.
.[/INDENT][/INDENT]

-- Inactive Products --

[INDENT] Apparel[/INDENT]
[INDENT][INDENT] Product 1
Product 2
.
.
.[/INDENT][/INDENT]

[INDENT] Books[/INDENT]
[INDENT][INDENT] Product 1[/INDENT][/INDENT]

-- On Sale Products --

[INDENT] Apparel[/INDENT]
[INDENT][INDENT] Product 1[/INDENT][/INDENT]

[INDENT] Books[/INDENT]
[INDENT][INDENT] Product 1
Product 2
.
.
.[/INDENT][/INDENT]

I don't think that your proposed solution would allow this... I'm not sure...

Thanks for your interest and for your response.

~Jordan

laserlight

I don't think that your proposed solution would allow this... I'm not sure...

It does the same thing as your original code, except that it flattens the array of categories into a list, thus reducing the number of queries by a factor proportional to the number of categories.

You may be able to reduce this further to just one query, but let's first see if what I showed you works.

coffejor

Great! I guess my PHP knowledge just isn't up to snuff to where I'd recognize that ( :-\ ). I will implement your proposed solution and let you know what the result is.

Thanks again for the help.

~Jordan

laserlight

Great! I guess my PHP knowledge just isn't up to snuff to where I'd recognize that ( :-\ ).

Actually, I think it is more SQL knowledge than PHP knowledge, since you have to see that the inner loop that goes through the entire categories array can be done by flattening the array to a list, then checking if category IN (list). Perhaps some "special" PHP knowledge is required to know how the flattening is done (i.e., the first line).

coffejor

Ok, I implemented your solution and got the same result. 😕

Here is the code after the new solution:

$category_list = "'" . implode("','", array_keys($categories)) . "'";
foreach ($status_array as $status=>$text) {
    $sql = "SELECT * FROM products WHERE status = '$status' AND category IN ($category_list)";
    $result = mysql_query($sql);

if (mysql_num_rows($result) > 0) {
    $content .= <<<EOF
        <h2>$text Products</h2>
        <p class="title">$category</p>
        <table summary="Table of $text Cub Club $info: Image, Description, and Price" class="maintable">
            <thead>
                <tr class="tableh1">
                    <th id="image" abbr="image" class="search">Image</th>
                    <th id="description" abbr="description" class="search">Description</th>
                    <th id="price" abbr="price" class="search">Price</th>
                </tr>
            </thead>
EOF;
        while ($row = mysql_fetch_array($result, MYSQL_ASSOC)) {
            $id = $row["id"];
            $image = $row["image"];
            $category = $row["category"];
            $sdescription = $row["sdescription"];
            $description = $row["description"];
            $price = $row["price"];
            $status = $row["status"];
            // Alternate the row colors
            $alternate = ($alternate == 'alternate' ? 'normal' : 'alternate');
            // If an image exists, get its dimensions
            if ($image2 = @getimagesize (URL."/products/thumbnail/small_{$row['image']}")) {
                $image_size = getimagesize (URL."/products/{$row['image']}");
            } else {
                $image_size = 0;
            }

        $content .= <<<EOF
            <tbody>
                <tr class="tableb">
                    <td headers="$class image" class="$alternate">
EOF;
            // If there's an image for the product, create and display a thumbnail with a link below it
            if ($image2 = @getimagesize (URL."/products/thumbnail/small_{$row['image']}")) {
                $content .= <<<EOF
                        <p class="image">
                        <img border="1" src="$root/products/thumbnail/small_$image" hspace="3" vspace="3" alt="" />
                        <br />
                        <a href="javascript: create_window ('$image',$image_size[0],$image_size[1])">View Pic</a>
                        </p>
EOF;
            } else { // If no image exist display a "No Image" flag in place of the thumbnail
                $content .= '<p class="image">No Image Available</p>';
            }

        $content .= <<<EOF
                    </td>
                    <td headers="$class description" class="$alternate">
                    <p class="title">$sdescription</p>$description
                    <p class="delete_button"><a href="$page?delete=$id">[x] Delete  </a><a href="$page?id=$id">[+] Edit</a></p>
                    </td>
                    <td headers="$class price" class="$alternate"><p class="price">$$price</p></td>
                </tr>
EOF;
        }

$content .= <<<EOF
                </tbody>
            </table>
            <br /><hr /><br />
EOF;
    }
}

With the arrays as follows:

$status_array = array('1' => 'Active','2' => 'Inactive','3' => 'On Sale');

$categories = array('Back Issues' => 'back_issues',
                    'Vertical Fin Decals' => 'fin_decals',
                    'Apparel' => 'apparel',
                    'Books and Media' => 'books',
                    'Memo Pads' => 'memo_pads',
                    'Pins & Tie Tacks' => 'pins');

Any more suggestions?

Thanks in advance.

~Jordan

laserlight

Ok, I implemented your solution and got the same result.

As in only the active products are listed?

Any more suggestions?

Double check the test data in your database. Also, remove this line since it is unnecessary and yet has potential to introduce confusion:

$status = $row["status"];

but it should not be the problem.

However, there is a bigger problem that I did not notice earlier. On each iteration of the while loop, you retrieve a product, not a category. This means that in order to list the products within a category, you need to sort the results returned, and then use some additional logic to determine when a category begins.

The sorting by category is easy. Change the query to:

$sql = "SELECT * FROM products WHERE status = '$status' AND category IN ($category_list) ORDER BY category";

Now, grouping by category is a little harder. The logic will be something like this:

if (mysql_num_rows($result) > 0) {
    // Append current status heading.
    $content .= '...';

$current_category = '';
while ($row = mysql_fetch_array($result, MYSQL_ASSOC)) {
    if ($row['category'] != $current_category) {
        // The current product is from a new category.
        $current_category = $row['category'];

        // Append current category heading.
        $content .= '...';
    }

    // Append current product details.
    $content .= '...';
}
}

coffejor

That did it! If you come to the Milwaukee area, I'm buying you a beer!

Thanks again for everything.

~Jordan

laserlight

Good to hear, and remember to mark this thread as resolved (if it is) using the thread tools 🙂

With only three status types, it probably does not matter that you go through a loop to execute SQL statements. However, note that you probably can reduce it to just one SQL statement, e.g.,

$sql = "SELECT * FROM products WHERE status IN ($status_list) AND category IN ($category_list) ORDER BY status, category";

Creating $status_list is like creating $category_list. The logic to begin a new status is like the logic to begin a new category.

Of course, if there will be no other status types other than those in the list, and there will be no other category types other than those in the list, then you can simplify to:

$sql = "SELECT * FROM products ORDER BY status, category";

That did it! If you come to the Milwaukee area, I'm buying you a beer!

I'm more of a wine drinker, but let's see if I come to your area first 🙂

coffejor

Just a note, I really didn't appreciate the person who felt the need to go through and delete all my products from the example page. I figured that all of us here are webmasters, php coders, and or web enthusiasts and, therefor, I could post a link to an unsecured version of the page. It saddens me that someone felt the need to do something so childish. I guess that I'll get back to work recreating all the products that were deleted...

~Jordan

laserlight

I figured that all of us here are webmasters, php coders, and or web enthusiasts and, therefor, I could post a link to an unsecured version of the page.

Unfortunately, that is rather naive, since you simply have no way of knowing if all of us are trustworthy (or even belong to any particular category of users), and to what extent.

It saddens me that someone felt the need to do something so childish.

It may even be the work of a bot, depending on how your form was designed and implemented.

Weedpacket

laserlight wrote:
It may even be the work of a bot, depending on how your form was designed and implemented.

Cf. http://thedailywtf.com/Articles/The_Spider_of_Doom.aspx