current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 3 down vote favorite

I need to calculate the calendar week of a given date. Constraints:

  • It should be properly calculated based on ISO 8601 if week starts on Monday
  • It should calculate based on US standards if the week starts on Sunday
  • It should be compact and fast, as the routine gets called very often
  • It should work even if the year start is not January (Fiscal Year starts on October for instance)

That's what I have currently:

Date.prototype.getWeek = function() { // ISO Week

    this.firstDay = 4;  // ISO 8601: Week with the first thursday which contains the 4th of January
    this.thursday = 4;  // 0 = Sunday OR Monday (if WeekStart = "M")

    if ( this.weekStart != 'M' )
    {
        this.firstDay = 1;  // switch to US norm: Week with the 1st of January
        this.thursday = 0;
    }

    // finding the date of the "thursday" in this week
    var donnerstag = function(datum, that) { 
        var Do = new Date(datum.getFullYear(), datum.getMonth(), 
                   datum.getDate() - datum.getDay() + that.thursday, 0, 0, 0);
        return Do;
    };

    // copy date, hourly times set to 0
    var Datum = new Date(this.getFullYear(),this.getMonth(),this.getDate(), 0, 0, 0); 

    var DoDat = donnerstag(Datum, this);        // the date of the first week
    var kwjahr = DoDat.getFullYear();           // the year of that date

    // diff from there to this date
    var DoKW1 = donnerstag(new Date(kwjahr, this.FYStart, this.firstDay), this); 
    //console.log(DoDat + " - " + DoKW1);

    // calculate the week
    var kw = Math.floor(1.5+(DoDat.getTime()-DoKW1.getTime())/86400000/7) 

    // adjust overflow
    if ( kw < 1 )
        kw = 52 + kw;

    return kw;
};

Is there any more compact algorithm or can this be optimized in terms of performance?

share|improve this question
1  
Hmm, the mix of German variable names and English comments is amusing. For better understanding: kw stands for the week of year, donnerstag and Do are Thursday. –  amon Feb 19 at 11:11
    
@amon: yepp, right. Reason for the mix: someone started (in German) and I took over. I am generally used to "code" in English entirely –  Axel Amthor Feb 19 at 11:17
1  
If your project does many date manipulations involving javascript, it might be useful to get a 3rd party library like moment.js. Of course, for a single function this would be abit much, but if you have many of these functions, it could be worth investing. –  Nate Kerkhofs Feb 19 at 12:03
    
@Nate: thanks for the tip. Way overloaded currently but good to know about it. –  Axel Amthor Feb 19 at 14:09

1 Answer 1

up vote 3 down vote

In my experience, Moment.js is somewhat too heavy for this simple task. Did some profiling not too long ago and the library does too much movement for this task. Maybe it's for robustness/scalability/compatibility purposes, but nevertheless, still heavy.

Straight to the point, check out this blog post which demonstrates his method of getting the ISO week. It appends an additional getWeek method to the native Date object. I have used this in one of my projects as substitute for Moment.js's ISO week function.

Date.prototype.getWeek = function () {  
    // Create a copy of this date object  
    var target  = new Date(this.valueOf());  

    // ISO week date weeks start on monday so correct the day number  
    var dayNr   = (this.getDay() + 6) % 7;  

    // ISO 8601 states that week 1 is the week with the first thursday of that year.  
    // Set the target date to the thursday in the target week  
    target.setDate(target.getDate() - dayNr + 3);  

    // Store the millisecond value of the target date  
    var firstThursday = target.valueOf();  

    // Set the target to the first thursday of the year  
    // First set the target to january first  
    target.setMonth(0, 1);  
    // Not a thursday? Correct the date to the next thursday  
    if (target.getDay() != 4) target.setMonth(0, 1 + ((4 - target.getDay()) + 7) % 7);  

    // The weeknumber is the number of weeks between the   
    // first thursday of the year and the thursday in the target week  
    return 1 + Math.ceil((firstThursday - target) / 604800000); // 604800000 = 7 * 24 * 3600 * 1000  
}

As for your code:

  • I suggest putting it in plain English as much as you can. Readability will be an issue for reviewers who prefer verbose, purposeful naming.

  • Compared to the proposed code I have put up, your version creates way more Date objects (3 at my count). This could be a significant bottleneck if you are calling this function over a set of tabulated data. This is going to eat memory and CPU time.

  • "Cloning" a Date is as simple as var clone = new Date(original.valueOf());. No need to individually call months, dates, etc. to setup the clone.

share|improve this answer
    
Thanks for the code and reco's. Your solution does not take into account different start of Fiscal Year and can not switch to US standard. Anyway +1 –  Axel Amthor Feb 20 at 7:35
    
Cloning the date is fine, BUT: in order to set the clock to midnight you need an additional call of setHours, ...Minutes, ...Seconds. Leaving the date at the current clock may result in rounding errors. –  Axel Amthor Feb 20 at 9:07

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.