1
\$\begingroup\$

I have these lines of code in my view:

<?php
    $count = count($product->getTags());
    $tagsStr = '';
    foreach($product->getTags() as $key => $tag){
        $tagsStr.= " " . $tag->getTag();
        if(($key == 0 && $count < 1) || ($key == 0 && $count >1 && $key != $count)){
            $tagsStr .= ',';
        }
    } ?>

It prints strings like:

Fish, Onions

Or

Fish, Onions, Eggs

All these items are stored in an ArrayCollection $product->getTags();.

I find that these are a lot of lines for completing something this simple. I was wondering if you have ideas on simplifying this code.

\$\endgroup\$
2
\$\begingroup\$
$tagsStr = implode(', ', array_map(function ($tag) { return $tag->getTag(); }, $product->getTags()->toArray()));
\$\endgroup\$
  • \$\begingroup\$ Sorry but that doesn't work since getTags(); returns an array coellection of tag objects. \$\endgroup\$ – sanders Nov 11 '13 at 18:33
  • \$\begingroup\$ You are right we need to use an array_map call or something similar to get the real tags. \$\endgroup\$ – Peter Kiss Nov 11 '13 at 18:38
  • \$\begingroup\$ It should be noted that this answer relies on lambda functions, which may not always be available. \$\endgroup\$ – mseancole Nov 14 '13 at 22:21
3
\$\begingroup\$

Peter Kiss's code works, but it's not fast.

<?php

$tags = $product->getTags();
$c    = count($tags);
$str  = null;
for ($i = 0; $i < $c; ++$i) {
    if ($str) {
        $str .= ", ";
    }
    $str .= $tags[$i]->getTag();
}

?>

Yes, it’s not short (and not much different to your solution) but it’s highly efficient and therefor the solution I’d go for.

\$\endgroup\$
  • \$\begingroup\$ Also much more legible and backwards compatible as it does not rely on lambda functions +1 \$\endgroup\$ – mseancole Nov 14 '13 at 22:21

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.