Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

For example I have 2 dates (year and month) like :

2013-03

2013-01 <-- Jan. current month.

2012-10

So my code is :

// Get current month and year
$m = date('m');
$y = date('Y');

// So I do something to get highest date so result is..
$highest = "2013-03";

// Current month and year
$current = $y."-".$m;


// So I do something to get lowest date so result is..
$lowest = "2012-10";


// My idea is get lower month from variables

function do_lower($date){
    $date = explode("-",$date);
    $y = $date[0];
    $m = $date[1];

    if($m != "01") {
        $m = $m-1;
        $m = sprintf('%02d',$m);
        return $y."-".$m;
    } else {
       return ($y-1)."-12";
    }

}


// This is my simple function to get result between high and low 

function different($h,$l) {
    global $data;
    $h = do_lower($h);
    if($h!=$l){
        $data[] = $h;
        different($h,$l);
    }
}

Then .. to do all functions like.. (JSON)

$data[] = $highest;
different($highest,$current);
$data[] = $current;
different($current,$lowest);
$data[] = $lowest;

So my result is correct

["2013-03","2013-02","2013-01","2012-12","2012-11","2012-10"]

Anyone can review my code ?

Thanks so much

share|improve this question
    
I'm confused. What's your end goal? Are you trying to list, in descending order, all dates between two given dates? –  Corbin Jan 9 '13 at 9:24
    
Yes you think right , Edited my code @Corbin –  l2aelba Jan 9 '13 at 9:27
add comment

1 Answer

up vote 3 down vote accepted

There's not really much to review here since it's such a short snippet, but a few things:


I tend to avoid manipulating dates as strings. With just a year-month format it's actually fine since there's no odd rules. It's generally a lot safer to use DateTime though (or even strtotime/date). The problem with dates and times are that they are not simple units. There are complex, ever-changing 'business rules' (of sorts) that surround dates and times and cause huge headaches.

As said though, you're fine with year-date format since there's no gotcha's.

Just for consistency sake though, I would work with a standardized date format and use a date-time specific API.

For example:

$start = new DateTime(date('Y-m')); //First of this month
$end = new DateTime('2013-03'); //March 1st
$oneDay = new DateInterval('P1D');
$dates = array();
for (; $end >= $start; $end->sub($oneDay)) {
    $dates[] = $end->format('Y-m');
}

$dates is then an array in the format you want. This code is quite ugly, but unfortunately I've never been able to figure out a way to manipulate dates in PHP that isn't ugly.


Here's a strtotime/date based version:

$start = strtotime(date('Y-m'));
$end = strtotime('2013-03');
$dates = array();
while ($end >= $start) {
    $dates[] = date('Y-m', $end);
    $end = strtotime(date('Y-m-d', $end) . ' +1 day');
}

Rather than the strtotime call, you could just use 864000 seconds. I'd have to think for a few minutes to make sure that won't have any DST mistakes though. Once again, a situation where I like to keep date operations as abstracted away as possible. 864000 is a lie since not all days have 24 hours (at least not in places that observe DST).


If I were to go with a string approach, your do_lower function is basically what I'd use. It's named rather poorly though since it has no mention of what it's 'lowering' or what 'lowering' even means. Also, I would probably use a strict comparison against 01.

The problem with a purely string based approach is that it's not very reusable. Your function has a very specific input format and a very specific output format. Anything else and it breaks.

If you pass around DateTime's instead, dates are no longer represented as a string. They're represented as a type that is much more appropriate for such a complicated system. They're represented as a type that is aware that they are a date.

share|improve this answer
    
Oh ! Nice answer , I will try your in my code :D thanks so much –  l2aelba Jan 9 '13 at 10:00
1  
@l2aelba Glad to help :) –  Corbin Jan 9 '13 at 10:02
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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