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

Expand BondFunctions::yield to accept dirty price as input too #648

Merged
merged 5 commits into from Nov 13, 2019

Conversation

igitur
Copy link
Contributor

@igitur igitur commented Jun 24, 2019

Currently BondFunctions::yield() accepts only the clean price to solve for the yield. This PR adds a parameter to specify whether the input price is either the dirty or clean price.

@lballabio lballabio added this to the 1.17 release milestone Aug 21, 2019
@lballabio
Copy link
Owner

I'd like the extra parameter to be some kind of enumeration instead of a bool, but that might require some more thought (ideally, I'd extract Callability::Price or part of it and put it in scope of class Bond instead). In the meantime, the boolean can do.

@igitur
Copy link
Contributor Author

igitur commented Aug 22, 2019

Oh, yes, I absolutely agree. I never new about the Callability::Price enumeration. In fact, I would then also recommend to change the BondHelper (and subclasses) constructor to use that enumeration instead of the last useCleanPrice parameter and mark the current one as obsolete.

@igitur
Copy link
Contributor Author

igitur commented Nov 12, 2019

Thanks @lballabio . What is your opinion about breaking changes, i.e. replacing the useCleanPrice parameter in BondHelper with a similar priceType variable? Or should we create yet another overloaded constructor?

@lballabio
Copy link
Owner

I would add another constructor and deprecate the existing.

lballabio added a commit that referenced this pull request Nov 13, 2019
@lballabio lballabio merged commit fc1feda into lballabio:master Nov 13, 2019
@igitur igitur deleted the yield-from-dirtyprice branch November 13, 2019 10:08
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.

None yet

2 participants