Start of month functions #2034
Start of month functions #2034
Conversation
|
Thanks—it looks like I should start dropping more hints in my posts 😄 |
ql/time/calendar.hpp
Outdated
| @@ -240,6 +246,14 @@ namespace QuantLib { | |||
| return impl_->isBusinessDay(_d); | |||
| } | |||
|
|
|||
| inline bool Calendar::isStartOfMonth(const Date& d) const { | |||
| return (d.month() != adjust(d-1, Preceding).month()); | |||
There was a problem hiding this comment.
I wonder if
return d == startOfMonth(d);
would be more or less efficient? It would be more clear for sure, and we wouldn't have two different implementations for two very related methods. The same goes for isEndOfMonth.
There was a problem hiding this comment.
We could do that, but would the test at
QuantLib/test-suite/calendars.cpp
Line 3383 in 6331e50
There was a problem hiding this comment.
It would still test consistency of the two methods, as if we were unaware of the implementation (which may change).
ql/time/calendar.hpp
Outdated
| inline bool Calendar::isEndOfMonth(const Date& d) const { | ||
| return (d.month() != adjust(d+1).month()); | ||
| return d == endOfMonth(d); |
There was a problem hiding this comment.
Ok, no, my bad—the previous implementation returns true if a date is the last business day of its month but also if it's a holiday after the last business day of the month. The check is used with this semantics while building schedules, which is why a few tests are failing. We should revert and comment it properly.
I'm not sure what to do with isStartOfMonth. I'm tempted to leave it as it is, even though it wouldn't be consistent with isEndOfMonth. Any thoughts?
There was a problem hiding this comment.
Oh, I see what you mean. But the same holds for the start of month too? E.g. both 1 May 2024 and 2 May 2024 should return true for isStartOfMonth() if you observe International Workers Day?
There was a problem hiding this comment.
That's what I'm not sure about. Ideally I'd return true only for the 2nd, but then it would be inconsistent with isEndOfMonth. I guess we can keep them consistent and document it.
In other news, we can keep the simpler implementation of isEndOfMonth if we write it as
return d >= endOfMonth(d);
i.e. with a >= instead of ==.
There was a problem hiding this comment.
I definitely prefer them consistent. How about we add overload functions if we strictly want to test only for the first/last business day of the month?
There was a problem hiding this comment.
If one wants a strict test they can write d == endOfMonth(d).
igitur
force-pushed
the
start-of-month-functions
branch
from
0ed1a81
to
5de635f
Compare
I noticed the comment in Luigi's blog about the lack of start-of-month functions. So here they are.