4
\$\begingroup\$

What this function does it's to return an array of remaining dates that are e.g. Mondays. Is this really the best way to do that?

function range(start, count) {
    return Array.apply(0, Array(count))
                .map(function (element, index) { 
                         return index + start;  
                     });
}

function GetAllRemainingDaysOfTheWeek(day)
{
    var today = new Date();
    var last = new Date(today.getFullYear(), 11, 31);
    today = new Date(today.getFullYear(), today.getMonth(), today.getDate());
    var diffInDays = (last.getTime() - today.getTime()) / (1000 * 3600 * 24);

    return range(1, diffInDays+1).map(function(x) {
       return new Date(today.getFullYear(), today.getMonth(), today.getDay()+x);
    }).filter(function(x){
       if(x.getDay() == 1) return true;
    });
}
\$\endgroup\$
6
\$\begingroup\$

Firstly, your code is buggy. You take a day argument, but you never use it for anything. You always return mondays.

But worse, you're incrementing dates like so:

new Date(today.getFullYear(), today.getMonth(), today.getDay()+x);

Spot the error? You're passing getDay()+x as the date argument. But getDay() returns the weekday number (0-6) - not the day-in-month.

So today, Thursday the 5th of November 20151, you're in effect saying:

new Date(2015, 10, 4 + x);

The year's correct, and the month's correct (January is zero), but the date is 1 day off. Thursday is day #4 in the 0-6 weekday scheme - but really, today's the 5th of November.

So if you were to run this on the, say, the 30th of November, you'd actually be using the 1st of November as your start date, since the 30th's a Monday. If you run it on a Sunday, getDay will return zero, meaning your start date will be the last of the previous month. And so on.

I also have to wonder why you've created a function, which builds an array just to hold a list of incrementing numbers, which you then map to dates. The simpler way of looping through incrementing numbers is a plain old for-loop.

Anyway, while calendars are complex, there's one thing that thankfully doesn't change: There are 7 days in a week. So you're creating 7 times more dates than you need, and filtering most of them away. You could consider simply skipping 7 dates at a time from the first occurrence.

Edit: The only thing in the above paragraph that holds true is that calendars are complex. As RobH points out in the comments, some pacific nations have been known to jump across the dateline. Presumably just to mess with programmers the world over!

Thirdly, don't mix calendar dates and raw time calculations like your division by milliseconds in a day. Switching back and forth between daylight savings time (aka winter/summer time) can cause all sorts of nonsense when mixing raw time and dates. Some calendar days are 25 hours long, some are 23. And since a date constructed from only year, month, and date sets its time to midnight, you risk skipping over a date, or repeating the same date twice.

Anyway, here's a simple solution that adds 1 day at a time until the year rolls over, storing dates that match what you're looking for:

function remainingDays(weekday) {
  var current = new Date,
      year = current.getFullYear(),
      dates = [];

  // a simple helper function
  function nextDay(date) {
    return new Date(date.getFullYear(), date.getMonth(), date.getDate() + 1);
  }

  // as long we're in the same year, keep adding 1 day,
  // and store the ones that match the weekday we're looking for
  while(current.getFullYear() === year) {
    if(current.getDay() === weekday) {
      dates.push(current);
    }
    current = nextDay(current);
  }

  return dates;
}

No need to build ranges, map, and filter them. Just a while loop and push.

Incidentally, if you want to learn more about why programming calendar and time things are just hideously complex, check out this video on youtube

1) Happy Guy Fawkes day everyone

\$\endgroup\$
  • 4
    \$\begingroup\$ +1 but there aren't always seven days between each Monday ;) bbc.co.uk/news/world-asia-16351377 \$\endgroup\$ – RobH Nov 5 '15 at 14:24
  • 2
    \$\begingroup\$ @RobH Aw, darn... I completely forgot about the dateline-hopscotch those nations have done. So, yeah... calendar stuff is just endless pitfalls. I'll update my answer, and thank you for reminding me \$\endgroup\$ – Flambino Nov 5 '15 at 14:27
  • \$\begingroup\$ I always remember that one because it happened on my birthday - well more specifically my birthday didn't exist in Samoa or Tokelau that year! \$\endgroup\$ – RobH Nov 5 '15 at 14:30
  • 1
    \$\begingroup\$ @RobH Jeez, hope you weren't there, expecting gifts and a party :) But it's kinda cool though - like being born on 29th Feb in a leap year, except much more rare \$\endgroup\$ – Flambino Nov 5 '15 at 17:17
  • 1
    \$\begingroup\$ @Jonah I tend to leave mistakes like that in place; if there are improvements, I just update, but outright mistakes I often leave in and comment on. Incidentally, I highly doubt JS actually handles the case of Samoa and Tokelau in 2011, since just looking at a date doesn't tell you what part of the world it's supposed to be used in. The date they skipped does exist everywhere else in the world, even for other places in the same time zone. So in order to really do this right, you need to involve geography and politics, which... ugh. \$\endgroup\$ – Flambino Nov 5 '15 at 19:04

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.