Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FuturesRateHelper supplying reference dates for yearFraction computation #1829

Merged
merged 8 commits into from
Jan 31, 2024

Conversation

PaulXiCao
Copy link
Contributor

Closes #1163.

The FuturesRateHelper constructors called yearFraction_ = dayCounter.yearFraction(earliestDate_, maturityDate_); without supplying reference dates. Those are needed to compute annual payment frequency for "Actual/365 Canadian" day count convention.

While working on this I factored out lots of repeated code within the constructors. Now the constructors are easier to understand and the actual fix for the issue can now be expressed as a one-line change.

@coveralls
Copy link

coveralls commented Nov 10, 2023

Coverage Status

coverage: 72.421% (+0.06%) from 72.361%
when pulling 96a0e12 on PaulXiCao:iss1163_CA365
into 772d19a on lballabio:master.

Copy link
Contributor

This PR was automatically marked as stale because it has been open 60 days with no activity. Remove stale label or comment, or this will be closed in two weeks.

@github-actions github-actions bot added the stale label Jan 11, 2024
@lballabio lballabio removed the stale label Jan 11, 2024
#include <utility>

#define APPLY_DATES(dates) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I did not miss something, you are always using this macro like

const FuturesRateHelperDates dates = InitializeDates(...);
APPLY_DATES(dates);

In this case it will be simpler to make InitializeDates into private methods that directly set the fields. Then you don't need this macro and the FuturesRateHelperDates struct.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use delegating constructors, the initialization code is no longer duplicated and can go in the constructors. I'll push the changes.

@lballabio lballabio added this to the Release 1.34 milestone Jan 31, 2024
@lballabio lballabio enabled auto-merge January 31, 2024 15:56
@lballabio lballabio merged commit daba5ab into lballabio:master Jan 31, 2024
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CA_365 day count issue
4 participants