moment-business-days
moment-business-days copied to clipboard
businessAdd doesn't work correctly all the time
The aim of this application is always to indicate the 16th business day of each month (means taking into account working days and public holidays). Some of the dates are correct and some are not, here is my actual result and what is the target if the dates were calculates correctly:
Month Actual Target Correct
Jan 22 22 yes
Feb 24 21 no
March 23 20 no
April 22 22 yes
May 22 22 yes
June 25 26 no
July 22 22 yes
Aug 21 24 no
This is the code I use (it is also available as jsfiddle):
myHolidays = [
{ holidayDate: "2020-06-01", description: "holiday 1" },
{ holidayDate: "2020-06-02", description: "holiday 2" },
{ holidayDate: "2020-06-03", description: "holiday 3" },
{ holidayDate: "2020-06-06", description: "weekend saturday" },
{ holidayDate: "2020-06-07", description: "weekend sunday" },
{ holidayDate: "2020-06-11", description: "holiday 6" },
{ holidayDate: "2020-06-13", description: "weekend saturday" },
{ holidayDate: "2020-06-14", description: "weekend sunday" },
{ holidayDate: "2020-06-20", description: "weekend saturday" },
{ holidayDate: "2020-06-21", description: "weekend sunday" },
{ holidayDate: "2020-06-27", description: "weekend saturday" },
{ holidayDate: "2020-06-28", description: "weekend sunday" },
];
moment.updateLocale('de', {
holidays: myHolidays.map(i => i.holidayDate),
holidayFormat: 'YYYY-MM-DD'
});
var startDate = moment("2020-01-01").startOf('month')
var endDate = moment("2020-12-01")
var eachMonthWith16 = [];
function generate16InEachMonth() {
let monthsList = []
// loop by months
while (endDate > startDate) {
monthsList.push(new Date(moment(this.startDate).businessAdd(15, 'days')))
startDate = startDate.add(1, "month").startOf('month')
}
return monthsList
}
console.log(generate16InEachMonth())
At a quick glance I'm guessing this has to do with months starting on a weekend or holiday. The counting should start from the first working day, right? For example in June 2020 should be on the 4th, then continue 5th, 8th and so on... i would need to step through a debugger but I suspect the first step is also counted.
Correct, it should start counting from the 4th in the June example. I have visualized the days:
The red marked days are holidays and the pink marked day should be the 16th business day of June, which is the 26th June taking in account the given holidays.
If I use businessAdd(16, 'days')
, then June is calculated correctly, but you get problems with other months, e.g. July should then be on 22, but is then 23.
@mcdado Could you already find out what causes this issue?
I created a demo sandbox . for my case the june's date is coming wrong. rest works fine. Although i know why it happened as i added 15 businessdays instead of 16.
The reason the code wouldn't work for all months is probably that the first day of the month is counted as a business day by default!
This is a tricky thing and should be mentioned in the docs, so developers can handle it e.g. like this:
var daysToAdd = (startDate.isBusinessDay()) ? 15 : 16
Or it should work differently by default. What do you think?
Sorry for answering only now. Could you please specify a runnable test with expected results? So it is clearer what should happen and what actually happens, and solve the problem.
I know this is old, but IMO the bug is in the application, not the library. You need to make sure you're adding 15 business days to the first business day of the month, rather than to the first day of the month.
startDate.businessAdd( 15 ), should (and does) return the 15th business day AFTER startDate, it does not matter that startDate is a business day or not.
function generate16InEachMonth() {
let monthsList = [];
// loop by months
while (endDate > startDate) {
const firstDay = moment( startDate );
if ( !firstDay.isBusinessDay()) {
firstDay.nextBusinessDay();
}
monthsList.push( new Date( firstDay.businessAdd( 15 )));
startDate.add( 1, "month" );
}
return monthsList;
}
I know this is old, but IMO the bug is in the application, not the library. You need to make sure you're adding 15 business days to the first business day of the month, rather than to the first day of the month.
Thank you for your feedback, if I understand correctly you feel that this is an expected behaviour from moment-business-days
, and one should be aware of it? In that case I could add some gotcha disclaimer in the README.
Yes, I feel this is expected behavior. If you want to add something to the README, that is fine, but I don't see the need.
I think of calling businessAdd( 15 )
as equivalent to calling nextBusinessDay()
15 times .
nextBusinessDay()
will always return a value at least 1 day after the original moment. It will never return the original moment. It does not (and should not) matter if the original moment is a business day or not.
So businessAdd(15)
also does not (and should not) behave differently when the original moment is business day or not.