tags:

views:

84

answers:

2

I have debugged this legacy code, and would like a sanity check on it.

The purpose of it is to allow someone to choose a delivery frequency for shipping a product. If someone wants their product Every Other Week, the system needs to determine if they should get an order next week, or two weeks from now. We call it A week, or B Week.

Keep in mind I did not write this, I am just trying to make sense of it and would like some help evaluating its accuracy:

if (date("l") == "Monday" ) {
            $start = 0;
        } else if (date("l") == "Tuesday" || date("l") == "Wednesday" || date("l") == "Thursday" || date("l") == "Friday" || date("l") == "Saturday"|| date("l") == "Sunday") {
        $start = -1;
    }

    // if changing to every other week set to next week's a/b-ness
    $a_week_tid = 34;
    $b_week_tid = 35;

    $every_other_week_frequency_id = 32;

    if ($delivery_frequency == $every_other_week_frequency_id) {
        $julian = (int) (strtotime('Monday +' . $start . ' week') / 86400);
        $julian_week = ($julian-4) / 7;
        if ($julian_week % 2) {
            $today_a_or_b = $b_week_tid;
            $next_week_a_or_b = $a_week_tid;
            $a_or_b_week_string = '(A Week)';
        } else {
            $today_a_or_b = $a_week_tid;
            $next_week_a_or_b = $b_week_tid;
            $a_or_b_week_string = '(B Week)';
        }
    } else {
        $next_week_a_or_b = NULL;
        $a_or_b_week_string = NULL;
    }

This code is not commented or documented. The part that confuses me is:

  1. Why is 4 subtracted from Julian, then divided by 7?
  2. If today is Monday, $julian_week is 2129, and 2129 % 2 evaluates TRUE. Is that correct?
  3. Is this even how it should be done? Can't I rewrite this using date('w') a lot easier?
+4  A: 

Yeah using date would totally be easier, plus it takes into account leap years, daylight saving time, all that extra stuff you don't want to have to deal with.

if (date('W')%2==1)

That's SOOOO much easier to maintain than the above.

Bob Baddeley
Very wrong -.- Doing a modulo operation with 1 on any integer will result in zero.
Aurel300
`x%1==0` always. maybe you wanted `%2` there.
Alin Purcaru
yep, fixed it. Thanks.
Bob Baddeley
modulo-1 won't allow you to determine if it's odd or even. it should be modulo-2
stillstanding
I thought this as well, but wasn't sure if there was something about php date calculation I didn't know.
Kevin
+1  A: 
  1. The ISO standard for week one in a year is that it is the week that the first Thursday of the year falls. This is the reason for the 4 subtracted from the Julian date. The week number is then found by dividing by 7.

  2. Again the ISO standard implies that week number cannot be greater than 53. I don't understand how your figure of 2129 can arise. However the div operator will not evaluate TRUE for this figure. Checking the div operator on the week number is the way of determining whether you are in week a or b. If it is before Thursday, it is quite likely that the number will be 1 less than you anticipate.

  3. The coding looks fairly good to me, though I have not stepped through all of it. It does look correct.

Chris Walton
This is kind of what I thought- but, does date('w') not take any of this into account?
Kevin
Sorry - I don't know about date('w').
Chris Walton
date('W') rather. W is the week number of the year.
Kevin