Loop with enclosing items every 'n' steps

Question

I'm listing elements with a foreach, and I would like to enclose items in something tag n by n:

$i = 0;
$open = $done = TRUE;
$n = 3;
$opening = "<div>";
$closing = "</div>";
$names = array("Doc", "Marty", "George", "Lorraine", "Einstein", "Biff"); 
foreach($names as $name) { /** $condition was not a bool. My fault. */
 if($open && !($i % n)) {
 print $opening;
 $open = FALSE;
 $done = !$open;
 } elseif(!($i % n)) $done = FALSE;
 /** print element. */
 print $name;
 if(!$open && !($i % n) && !$done) {
 print $closing;
 $open = TRUE;
 $done = !$open;
 }
 $i++;
}

Do you think this snippet can be improved? It would be better to have fewer check variables such as $open or $done.

• greatwolf: @albert can you also add what n and the condition is suppose to represent? Does it get it's value from another part of the code?
• edorian: Can you provide as working example ? (Without parser errors). From its current state i pretty much have no clue that code is supposed to be doing (or at least it's nothing i think i can improve apon in a meaningfull way) . i, open and done are php variables ? ($ sign ?)
• Donovan: I am very sorry. I updated the question.
• RobertPitt: } elseif(!(i % n)) done = FALSE; is not PHP!, }elseif(!($i % $n)) $done = FALSE; is, this is a job for StackOverflow before you it can be improved
• edorian: open and close still me the $ signs, saidly i can't edit it myself (and i don't want to put in a question just for that.) Thanks for the edit ! Much clearer now.

Answer 1

One problem with your solution is that it won't print the last closing </div> tag if there are less than $n elements inside the last <div>.

An approach which fixes that problem and also simplifies the conditional logic for deciding when to print the tags is to chunk the array first using array_chunk and then iterate over the chunks. Now we only need one boolean variable to remember which chunks to surround in tags.

function alternate($names, $n, $opening, $closing) {
 $tag = TRUE;
 foreach(array_chunk($names, $n) as $chunk) {
 if($tag) {
 print $opening;
 }
 foreach($chunk as $name) {
 print $name;
 }
 if($tag) {
 print $closing;
 }
 $tag = !$tag;
 }
}

Answer 2

Alternate solution without the overhead of the array_chunk call. (Don't get me wrong, I love array_chunk and use it for a lot of things, but this isn't one where it's needed.)

$array = getData(...);
$n = 5; //or whatever you want to chunk into
echo "<div>"; //always open one
$lcv = 1; // loop count variable
foreach ($array as $key => $val) {
 //format $key/$val as needed here...
 if ($lcv++ % $n == 0) { //every Nth item
 echo "</div><div>"; //close old one and open new one
 }
}
echo "</div>"; //always close the last one.

Worst case here is you might have an empty <div></div> block at the end if you had an exact multiple of n.... but that's not really that big a deal 99.999% of the time. :) If it is... then you can catch it like so:

$array = getData(...);
$num = count($array);
$n = 5; //or whatever you want to chunk into
echo "<div>"; //always open one
$lcv = 1; // loop count variable
foreach ($array as $key => $val) {
 //format $key/$val as needed here...
 if ($lcv++ % $n == 0 && $lcv < $num) { // every Nth item, unless we're done
 echo "</div><div>"; //close old one and open new one
 }
}
echo "</div>"; //always close the last one.

(Of course you can use print, echo, or ?>...<? tags... whatever you want to actually do all the output, doesn't matter.)

And to be honest, I'd probably add one more case to it:

$array = getData(...);
$num = count($array);
if ($num == 0) {
 echo "<div>No results?</div>";
} else {
 $n = 5; //or whatever you want to chunk into
 echo "<div>"; //always open one
 $lcv = 1; // loop count variable
 foreach ($array as $key => $val) {
 //format $key/$val as needed here...
 if ($lcv++ % $n == 0 && $lcv < $num) { // every Nth item, unless we're done
 echo "</div><div>"; //close old one and open new one
 }
 }
 echo "</div>"; //always close the last one.
}

Just to cover the case that whatever search it is didn't actually return anything.