Outer/inner loop problem

Hi guys,

there’s something wrong with my math on this loop, it is a scrolling banner which loops through an array of different manufacturers and displays them on the screen, the size of manufacturers will vary but I always want it to display 6 of them in each rotation, can anyone spot the error? At the moment it is repeating certain manufacturers, not sure why, these inner/outer loop things often confuse me as I’m more of a designer.

thanks

  <?php for ($i = 0; $i < sizeof($manufacturers); $i = $i + 6) { ?>
    <div>
      <?php for ($j = $i; $j < sizeof($manufacturers); $j++) { ?>
      <a href="<?php echo str_replace('&', '&amp;', $manufacturers[$j]['href']); ?>" class="scroll_item">

      <?php if (isset($manufacturers[$j])) { ?>

<img src="<?php echo "image/".$manufacturers[$j]['manufacturer_image']; ?>" title="<?php echo $manufacturers[$j]['name']; ?>" alt="<?php echo $manufacturers[$j]['name']; ?>" height="80"/>

<?php } ?>

</a>
      <?php } ?>
    </div>
    <?php } ?>

Hi,

Can you dump an example of your $manufacturers multidimensional array?

You only ever want 6 to display so is $manufacturers randomized in some way before being processed; otherwise how else do you get different $manufacturer groups of 6 each time you run the banner?

Steve

Hi thanks for your help again Steve, here is the page, I have done a print_r above the slider :wink:

http://www.wavemobilephones.com/index.php?route=common/home

Here is the controller file for that module:



<?php  
class ControllerModuleManufacturers extends Controller {
	protected function index() {
		$this->language->load('module/manufacturers');	
		
		$this->data['heading_title'] = $this->language->get('heading_title');
		$this->data['text_select'] = $this->language->get('text_select');
		
		if (isset($this->request->get['manufacturer_id'])) {
			$this->data['manufacturer_id'] = $this->request->get['manufacturer_id'];
		} else {
			$this->data['manufacturer_id'] = 0;
		}
		
		$this->load->model('catalog/manufacturer');
	        $this->load->model('catalog/category');
		$this->load->model('tool/seo_url'); 
		 
		$this->data['manufacturers'] = array();
		
		$results = $this->model_catalog_manufacturer->getManufacturers();
	     	$cats =    $this->model_catalog_category->getCategories(); //returns parent category 
                $start_key = $cats[0]['category_id'];   // get first child category, will always be Mobile Brands anyway
                $child = $this->model_catalog_category->getCategories($start_key); //when you pass this a key
              
              for($i = 0; $i < sizeof($child ); $i++)
              {
                 if($child[$i]['name'] == $results[$i]['name'])
                  {
                     $results[$i]['manufacturer_id'] = $child[$i]['category_id'];
                    // echo "IN categories is :".$child[$i]['name']." and in manufacturers is :".$results[$i]['name']."<br/>";
                  } 
              }


		foreach ($results as $result) {
                        
			$this->data['manufacturers'][] = array(
				'manufacturer_image' => $result['image'],
				'manufacturer_id' => $result['manufacturer_id'],
				'name'            => $result['name'],
                                'href'           => $this->model_tool_seo_url->rewrite(HTTP_SERVER . 'index.php?route=product/category&path=60_' . $result['manufacturer_id'])				
			);
		} 


		
		$this->id = 'manufacturers';
		
		if (file_exists(DIR_TEMPLATE . $this->config->get('config_template') . '/template/module/manufacturers.tpl')) {
			$this->template = $this->config->get('config_template') . '/template/module/manufacturers.tpl';
		} else {
			$this->template = 'default/template/module/manufacturers.tpl';
		}
		
		$this->render(); 
	}
}
?>

I opted not to use the predefined manufacturers field(for various reasons) so I wrote an algorithm to take the names of the categories which were by manufacturer anyway.

Here’s how I would do it.

	<div>
	<?php
	foreach($manufacturers as $key => $manufacturer){
		if($key % 6 == 0){
			echo '</div><div>';
		}
		$href = str_replace('&', '&amp;', $manufacturer['href']);
		$name = $manufacturer['manufacturer_name'];
		$image = $manufacturer['manufacturer_image'];
		echo <<<MANUFACTURER
			<a href="{$url}" class="scroll_item">
				<img src="image/{$image}" title="{$name}" alt="{$name}" height="80"/>
			</a>
MANUFACTURER;
		//the above line must be unindented. For more information check out "heredoc"
	}
	?>
	</div>

Hi,

Do you understand what Jake has shown? It is a cleaner way than you were trying and more efficient too.

Even being more of a designer rather than a PHP guru, you should become familiar with the Heredoc syntax as it allows you to freely mix single and double quotes as well as have php variables mixed in. It leads to easier reading and less typing.

BTW nice @Jake_Arkinstall ;

Ok thanks both I see what you mean about the heredoc syntax, can’t believe I haven’t come across it before on forums etc.
It’s much easier than using backslashes or single/double especially when you have lots of php variables to insert at different places in a string.

So let me get this right guys if I use heredoc I must always have a new line after echo <<<MANUFACTURER and then make sure that the closing MANUFACTURER is fully indented?

Also you have used MANUFACTURER in this instance but that is just a marker right? In theory it could be called anything except for reserved system words?

I am also a little confused by this line of code:

    if($key % 6 == 0){
        echo '&lt;/div&gt;&lt;div&gt;';
    } 

I know the modulus syntax which flags when we reach 6 but why would I put a closing <div> and then an opening one? Isn’t that leaving an empty div at the end for no reason because the looped content is wrapped in a div so you would get:

<div> // start div
//looped content
reaches 6 so outputs
</div><div>
</div> // empty div??

Wouldn’t it just be better to exit the loop?

thanks so much tho guys really interesting I am learning all the time with php, it’s a great language because there are just so many interesting ways of doing the same thing :slight_smile: