3
\$\begingroup\$

I am trying to implement a function groupWeeks(dates) using reduce that allows me to group an array of objects { date: '2019-03-11', count: 8 } by weeks.

The following code works fine. Is there a way to refactor it? It seems to me that my solution is unnecessarily complex. Please ignore Date.prototype.getWeekNumber and getWeekStart(date).

const dates = [
  { date: '2019-02-24', count: 10 },
  { date: '2019-02-25', count: 11 },
  { date: '2019-02-26', count: 12 },
  { date: '2019-03-09', count: 8 },
  { date: '2019-03-10', count: 7 },
  { date: '2019-03-11', count: 6 },
  { date: '2019-04-14', count: 3 },
  { date: '2019-04-15', count: 2 },
  { date: '2019-04-16', count: 4 },
  { date: '2019-04-22', count: 5 }
];

/**
 * Returns the week number for this date.  dowOffset is the day of week the week
 * "starts" on for your locale - it can be from 0 to 6. If dowOffset is 1 (Monday),
 * the week returned is the ISO 8601 week number.
 * @param int dowOffset
 * @return int
 */
Date.prototype.getWeek = function(dowOffset) {
  /*getWeek() was developed by Nick Baicoianu at MeanFreePath: http://www.epoch-calendar.com */

  dowOffset = typeof dowOffset == 'int' ? dowOffset : 0; //default dowOffset to zero
  var newYear = new Date(this.getFullYear(), 0, 1);
  var day = newYear.getDay() - dowOffset; //the day of week the year begins on
  day = day >= 0 ? day : day + 7;
  var daynum =
    Math.floor(
      (this.getTime() -
        newYear.getTime() -
        (this.getTimezoneOffset() - newYear.getTimezoneOffset()) * 60000) /
        86400000
    ) + 1;
  var weeknum;
  //if the year starts before the middle of a week
  if (day < 4) {
    weeknum = Math.floor((daynum + day - 1) / 7) + 1;
    if (weeknum > 52) {
      nYear = new Date(this.getFullYear() + 1, 0, 1);
      nday = nYear.getDay() - dowOffset;
      nday = nday >= 0 ? nday : nday + 7;
      /*if the next year starts before the middle of
           the week, it is week #1 of that year*/
      weeknum = nday < 4 ? 1 : 53;
    }
  } else {
    weeknum = Math.floor((daynum + day - 1) / 7);
  }
  return weeknum;
};

function getWeekStart(date) {
  var offset = new Date(date).getDay();
  return new Date(new Date(date) - offset * 24 * 60 * 60 * 1000)
    .toISOString()
    .slice(0, 10);
}

function groupWeeks(dates) {
  const groupsByWeekNumber = dates.reduce(function(acc, item) {
    const today = new Date(item.date);
    const weekNumber = today.getWeek();

    // check if the week number exists
    if (typeof acc[weekNumber] === 'undefined') {
      acc[weekNumber] = [];
    }

    acc[weekNumber].push(item);

    return acc;
  }, []);

  return groupsByWeekNumber.map(function(group) {
    return {
      weekStart: getWeekStart(group[0].date),
      count: group.reduce(function(acc, item) {
        return acc + item.count;
      }, 0)
    };
  });
}

console.log(groupWeeks(dates));

\$\endgroup\$
1
\$\begingroup\$

You said to ignore getWeekStart(), but I'm afraid I can't do that. It appears that the intention of getWeekStart() is to take a date string (in YYYY-MM-DD form) and find the Sunday that starts the week (in YYYY-MM-DD form). However, depending on the time zone in which the code is executed, it may actually find the Monday, or possibly a Saturday instead.

The problem is that

  • new Date("YYYY-MM-DD") interprets the date as a UTC timestamp:

    Note: parsing of date strings with the Date constructor (and Date.parse, they are equivalent) is strongly discouraged due to browser differences and inconsistencies. Support for RFC 2822 format strings is by convention only. Support for ISO 8601 formats differs in that date-only strings (e.g. "1970-01-01") are treated as UTC, not local.

  • date.getDay() returns the day of the week for the specified date according to local time, where 0 represents Sunday.

  • date.toISOString() works in UTC:

    The toISOString() method returns a string in simplified extended ISO format (ISO 8601), which is always 24 or 27 characters long (YYYY-MM-DDTHH:mm:ss.sssZ or ±YYYYYY-MM-DDTHH:mm:ss.sssZ, respectively). The timezone is always zero UTC offset, as denoted by the suffix "Z".

For the code to be reliably correct, you must work consistently in UTC — in other words, call date.getUTCDay() instead.

Furthermore, I'd rename getWeekStart() to weekStart(). The "get" prefix implies that you are fetching something that already exists. However, you are actually computing something, so the prefix is superfluous, perhaps even misleading.

\$\endgroup\$
  • \$\begingroup\$ I really appreciate looking into details. I did not realize it could be a problem. \$\endgroup\$ – John John Apr 25 at 6:42

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.