6
\$\begingroup\$

Just thinking if using code like this

<?php

abstract class helper
{
    private static $_cycles = array();

    public static function isOdd($v)
    {
        return (0 == ($v % 2)) ? false: true;
    }

    public static function isEven($v)
    {
        return !self::isOdd($v);
    }

    public static function cycle($odd, $even)
    {
        $trace = debug_backtrace();
        $trace = $trace[0];
        $cycle = crc32(serialize($trace));

        if (!isset(self::$_cycles[$cycle]))
        {
            self::$_cycles[$cycle] = 1;
        }

        return (self::isOdd(self::$_cycles[$cycle]++)) ? $odd : $even;
    }
}

for featues like this

<?php foreach ($data as $record): ?>
    <p class="<?php echo helper::cycle('oddCss', 'evenCss'); ?>"><?php echo $record; ?></p>
<?php endforeach; ?>

is not overcoded

PS
Easy of usage (by html/css coders) is more important then performance in this particular case

UPDATE:
As @Geoffrey mentioned i didn't tell before that same 'oddCss', 'evenCss' pair may be used several times on one page (so basically within one request)

\$\endgroup\$
2
  • \$\begingroup\$ I'm guessing that the second code block is what the HTML/CSS people will need to write? \$\endgroup\$
    – Michael K
    Mar 25, 2011 at 18:12
  • \$\begingroup\$ @Michael that's right \$\endgroup\$ Mar 25, 2011 at 18:26

5 Answers 5

3
\$\begingroup\$

Well, using debug_backtrace for it (clearly not the fastest function in the town) is a huge overkill. What about passing id? Advantage, besides of performance, is that you can use the same counter in different places.

Code:

<?php

abstract class helper
{

    protected static $_cycles = array();

    public static function cycle($cycle_id, $odd, $even)
    {

        self::$_cycles[$cycle_id] = isset(self::$_cycles[$cycle_id]) ? !self::$_cycles[$cycle_id] : true;

        return self::$_cycles[$cycle_id] ? $odd : $even;
    }

}

Usage:

<?php foreach ($data as $record): ?>
    <p class="<?php echo helper::cycle('mycycle', 'oddCss', 'evenCss'); ?>"><?php echo $record; ?></p>
<?php endforeach; ?>
// Data from another source, but continuous display
<?php foreach ($data2 as $record): ?>
    <p class="<?php echo helper::cycle('mycycle', 'oddCss', 'evenCss'); ?>"><?php echo $record; ?></p>
<?php endforeach; ?>
\$\endgroup\$
1
  • \$\begingroup\$ This solution is perfect and its quite similar to my first attempt which was passing array idx (foreach ($data as $k => $v)) and checking only odd/even. Passing simple $cycle_id is real compromise between backend and frontend programmers. Thanks for fresh look and great idea about this question! \$\endgroup\$ Mar 25, 2011 at 21:32
5
\$\begingroup\$

Since this is a code review site, let me point out that the following is an abomination:

return (0 == ($v % 2)) ? false: true;

The constants are redundant. Much simpler:

return (0 != ($v % 2));

(The parentheses are also redundant but that’s unrelated.)

\$\endgroup\$
3
  • 1
    \$\begingroup\$ I like the parentheses, though. I think it clarifies the statement quite a bit. \$\endgroup\$
    – Michael K
    Mar 25, 2011 at 18:11
  • 3
    \$\begingroup\$ The original returned true for even, false for odd. The above reverses those. Try return (bool) ($v % 2) or if you don't like the case, return ($v % 2 == 1). \$\endgroup\$ Mar 26, 2011 at 9:07
  • \$\begingroup\$ @David Good call. I should have paid better attention. \$\endgroup\$ Mar 26, 2011 at 9:35
3
\$\begingroup\$

Something a bit simpler (without logging):

class Cycler {
   $evenFlag = true;

   /* Returns the next value in an even/odd series */
   function next($odd, $even) {
      $evenFlag = !evenFlag;

      return $evenFlag? $even: $odd;
   }
}

Usage:

<?php Cycler cycler = new Cycler(); ?>
<?php foreach ($data as $record): ?> 
   <p class="<?php echo cycler.next('oddCss', 'evenCss'); ?>"><?php echo $record; ?></p>
<?php endforeach; ?>

You could also add a reset() if you wanted to use the Cycler multiple times.

\$\endgroup\$
2
  • \$\begingroup\$ Just remember that when sth is simple for programmers (e.g. .next() .reset() methods) its not always easy for html/css coders \$\endgroup\$ Mar 25, 2011 at 21:30
  • \$\begingroup\$ That is perhaps true for reset. I don't think next is too much of a challenge. However it isn't the most descriptive - maybe flip or switch? ideas... \$\endgroup\$
    – Michael K
    Mar 26, 2011 at 0:18
1
\$\begingroup\$

This is clearly over-engineered. The isOdd and isEven method are useless since they're just the result of fmod($i, 2)

I'm guessing you're using the crc part to keep track of specific cycles for $odd/$even, couples, but this is not needed. Instead, you can just hash the arguments and keep a counter for that hash.

Last, I think static are an heresy, and I'd much like an instanciated helper.

I would have written something like that, assuming $odd and $even are always strings (not tested):

<?php

class helper
{
  private $cycles = array();

  function cycle($odd, $even)
  {
    $hash = md5($odd.$even);

    if (!isset($this->cycles[$hash]))
    {
      $this->cycles[$hash] = 0;
    }

    return fmod($this->cycles[$hash]++, 2) ? $odd : $even;
  }
}

$helper = new helper();

$helper->cycle('foo', 'bar');
\$\endgroup\$
3
  • \$\begingroup\$ your solution fails when i need 2 or more cycles with the same $odd $even values on the same page \$\endgroup\$ Mar 25, 2011 at 18:25
  • \$\begingroup\$ oh I see. you should have told that in your question :-) Anyway, with that in mind, I don't see another solution right now (but my other remarks still apply :p) \$\endgroup\$ Mar 25, 2011 at 18:30
  • \$\begingroup\$ Yea, my bad. Thanks for pointing that out (updated) and thanks for the tips also \$\endgroup\$ Mar 25, 2011 at 18:34
0
\$\begingroup\$

I think CodeIgniter has something like this (yup, the alternator() helper):

function Cycle()
{
    static $i = 0;

    if (func_num_args() > 0)
    {
        return func_get_arg($i++ % func_num_args());
    }

    return $i = 0;
}

Works for a variable number of arguments, which might be useful in certain scenarios.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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