Long-lived projects can have… interesting little corners. Choices made 13 years ago can stick around, either because they work well enough, or because, well, every change breaks somebody's workflow.
Today's anonymous submitter was poking around the code base of a large, long-lived JavaScript framework. In a file, not modified since 2007, but still included in the product, they found this function.
_getAdjustedDay: function(/*Date*/dateObj){
//FIXME: use mod instead?
//summary: used to adjust date.getDay() values to the new values based on the current first day of the week value
var days = [0,1,2,3,4,5,6];
if(this.weekStartsOn>0){
for(var i=0;i<this.weekStartsOn;i++){
days.unshift(days.pop());
}
}
return days[dateObj.getDay()]; // Number: 0..6 where 0=Sunday
}
Look, this is old JavaScript, it's date handling code, and it's handling an unusual date case, so we already know it's going to be bad. That's not a surprise at all.
The core problem is, given a date, we want to find out the day of the week it falls on, but weeks don't have to start on Sunday, so we may need to do some arithmetic to adjust the dates. That arithmetic, as the FIXME
comment helpfully points out, could easily be done with the %
operator.
Someone knew the right answer here, but didn't get to implementing it. Instead, we have an array of valid values. To calculate the offset, we "roll" the array using a unshift(pop)
combo- take the last element off the array and plop it onto the front. We also have a bonus unnecessary "if" statement- the "for" loop would have handled that.
This isn't the first time I've seen "populate an array with values and roll the array instead of using mod", and it probably won't be the last. But there's also a bonus WTF here. This function is invoked in _initFirstDay
.
_initFirstDay: function(/*Date*/dateObj, /*Boolean*/adj){
//adj: false for first day of month, true for first day of week adjusted by startOfWeek
var d = new Date(dateObj);
if(!adj){d.setDate(1);}
d.setDate(d.getDate()-this._getAdjustedDay(d,this.weekStartsOn));
d.setHours(0,0,0,0);
return d; // Date
}
So, first off, this function does two entirely different things, depending on what you pass in for adj
. As the comment tells us, if adj
is false, we find the first day of the month. If adj
is true, we find the first day of the week relative to startOfWeek
. Unfortunately, I'm not sure that comment is entirely correct, because whether or not adj
is false, we do some arithmetic based on _getAdjustedDay
. So, if you try this for a date in November 2020, with weeks starting on Sunday, you get the results you expect- because November 1st was a Sunday. But if you try it for October, the "first day" is September 27th, not October 1st.
Maybe that's by intent and design. Maybe it isn't. It's hard to tell from the comment. But the real bonus WTF is how they call this._getAdjustedDay
here- passing in two parameters. To a function which only expects one. But that function does use the value passed in anyway, since it's a property of the class.
Even code that we can safely assume is bad just from knowing its origins can still find new ways to surprise us.