7
\$\begingroup\$

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.

\$\endgroup\$
7
  • 1
    \$\begingroup\$ @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? \$\endgroup\$
    – greatwolf
    Jan 20 '11 at 11:03
  • 3
    \$\begingroup\$ 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 ?) \$\endgroup\$
    – edorian
    Jan 20 '11 at 11:11
  • 1
    \$\begingroup\$ I am very sorry. I updated the question. \$\endgroup\$
    – Donovan
    Jan 20 '11 at 11:34
  • 2
    \$\begingroup\$ } 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 \$\endgroup\$
    – RobertPitt
    Jan 20 '11 at 11:39
  • 1
    \$\begingroup\$ 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. \$\endgroup\$
    – edorian
    Jan 20 '11 at 11:40
9
\$\begingroup\$

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;
    }
}
\$\endgroup\$
0
3
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.