1
\$\begingroup\$

I am trying to simplify the amount of code required here using arrays, I feel like my foreach loop is still excessively long. Any advice would be appreciated.

$date_list = Array(
Array( 'age', "$birthday", 'now' ),
Array( 'membership', "$datejoined", 'now' ),
Array( 'promoted', "$lastpromo", 'now' ),
);

foreach ( $date_list as $date_set ) {
$var = $date_set[0];
$start = $date_set[1];
$end  = $date_set[2];
$datetime1 = date_create($start);
$datetime2 = date_create($end);
$interval = date_diff($datetime1, $datetime2);
if ( substr_count( $var, 'age' ) > 0 ){
    $age = $interval->format('%y');
}
elseif ( substr_count( $var, 'membership' ) > 0 ){
    $years = $interval->format('%y');
    $months = $interval->format('%m');
    $membership = floor(($years*12)+$months);
    if($membership > 1){
        $suffix = 'Months';
    }
    elseif($membership == 1){
        $suffix = 'Month';
    }
    else{
        $membership = "< 1";
        $suffix = 'Month';
    }
}
elseif ( substr_count( $var, 'promoted' ) > 0 ){
    $years = $interval->format('%y');
    $months = $interval->format('%m');
    $test = $interval->format('%a');
    $promoted = floor(($years*12)+$months);
    if($promoted > 1){
        $suffix = 'Months ago';
    }
    elseif($promoted == 1){
        $suffix = 'Month ago';
    }
    else{
        $promoted = "< 1";
        $suffix = 'ago';
    }
}

}

\$\endgroup\$
  • \$\begingroup\$ Ugh. Firstly, use switch statements. Why are you using substr_count when you could use switch? You put the data there, you know it is clean. If this is just test code and in reality it would have input from elsewhere, just clean it up either before you put it into the array or after you extract it. \$\endgroup\$ – itsbruce Aug 16 '13 at 23:30
  • \$\begingroup\$ Secondly, your question is ambiguous. Do you mean that you tried to use arrays to improve the code, but need advice on how to complete that, or that you want to improve this code, which just happens to use arrays? If you can be clearer about this, and about why you are using arrays, that will help \$\endgroup\$ – itsbruce Aug 16 '13 at 23:33
  • \$\begingroup\$ Finally, a little more context for this code snippet would help. \$\endgroup\$ – itsbruce Aug 16 '13 at 23:37
1
\$\begingroup\$

It's hard to suggest simplifications without a greater understanding of the problem being solved. However, this is perhaps a bit more readable:

$dates = array(
    array('type' => 'age',        'start' => $birthday,   'stop' => 'now' ),
    array('type' => 'membership', 'start' => $datejoined, 'stop' => 'now' ),
    array('type' => 'promoted',   'start' => $lastpromo,  'stop' => 'now' ),
);

foreach($dates as $date)
{
    $start = date_create($date['start']);
    $stop  = date_create($date['stop']);
    $interval = date_diff($start, $stop);

    switch($date['type'])
    {
        case 'age':
            $age = $interval->format('%y');
            break;

        case 'membership':
            $years      = $interval->format('%y');
            $months     = $interval->format('%m');
            $membership = floor(($years*12)+$months);

            if($membership >  1) { $suffix = 'Months'; }
            if($membership == 1) { $suffix = 'Month'; }
            if($membership <  1) { $suffix = 'Month'; $membership = '< 1'; }
            break;

        case 'prompted':
            $years    = $interval->format('%y');
            $months   = $interval->format('%m');
            $test     = $interval->format('%a');
            $promoted = floor(($years*12)+$months);

            if ($promoted  > 1) { $suffix = 'Months ago'; }
            if ($promoted == 0) { $suffix = 'Month ago';  }
            if ($promoted  < 1) { $suffix = 'ago'; $promoted = '< 1'; }
            break;
    }
}

?

|improve this answer|||||
\$\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.