Supply MakeVanillaSwap builder to SwapRateHelper
#1932
Conversation
PaulXiCao
force-pushed
the
iss1393_SwapRateHelper
branch
from
eca1451
to
388ecb5
Compare
PaulXiCao
changed the title
MakeVanillaSwap builder to SwapRateHelper
| @@ -112,6 +112,8 @@ namespace QuantLib { | |||
| ext::optional<BusinessDayConvention> paymentConvention_; | |||
|
|
|||
| ext::shared_ptr<PricingEngine> engine_; | |||
|
|
|||
| friend class SwapRateHelper; | |||
There was a problem hiding this comment.
This is added to enable access within SwapRateHelper::SwapRateHelper(const MakeVanillaSwap& swapBuilder) to swapBuilder.forwardStart_.
| @@ -347,6 +358,7 @@ namespace QuantLib { | |||
| Handle<YieldTermStructure> discountHandle_; | |||
| RelinkableHandle<YieldTermStructure> discountRelinkableHandle_; | |||
| ext::optional<bool> useIndexedCoupons_; | |||
| bool swapBuiltViaGivenBuilder; | |||
There was a problem hiding this comment.
I believe this issue "'SwapRateHelper::swapBuiltViaGivenBuilder' is never used" from "Codacy Static Code Analysis" is a false positive. The member is used, e.g. in ratehelpers.cpp:l.681 SwapRateHelper::setTermStructure().
| @@ -347,6 +358,7 @@ namespace QuantLib { | |||
| Handle<YieldTermStructure> discountHandle_; | |||
| RelinkableHandle<YieldTermStructure> discountRelinkableHandle_; | |||
| ext::optional<bool> useIndexedCoupons_; | |||
| bool swapBuiltViaGivenBuilder; | |||
There was a problem hiding this comment.
We are supporting two different ctor types
- one accepting arguments (e.g.
Period tenor) to built up theMakeVanillaSwap - another one accepting directly the builder, i.e.
MakeVanillaSwap swapBuilder
Thus, different member variables are used. We could represent this in code (e.g. via optional, variant). Here I used a version of this single boolean for minimal code changes.
| SwapRateHelper(const Handle<Quote>& rate, | ||
| const MakeVanillaSwap& swapBuilder, | ||
| Handle<Quote> spread = {}, | ||
| Pillar::Choice pillar = Pillar::LastRelevantDate, | ||
| Date customPillarDate = Date()); | ||
| SwapRateHelper(Rate rate, | ||
| const MakeVanillaSwap& swapBuilder, | ||
| Handle<Quote> spread = {}, | ||
| Pillar::Choice pillar = Pillar::LastRelevantDate, | ||
| Date customPillarDate = Date()); |
There was a problem hiding this comment.
There are multiple pairs of ctors which differ only in Rate rate vs const Handle<Quote>& rate. Should we try to find a way to reduce this duplication? I could imagine there might be a way to define a macro which produces both declarations in the header. The implementation in the cpp would stay as is.
| void SwapRateHelper::setTermStructure(YieldTermStructure* t) { | ||
| if (swapBuiltViaGivenBuilder) | ||
| return; |
There was a problem hiding this comment.
I did not see a straight forward way to access the term structure when the MakeVanillaSwap builder was given in the ctor. Any ideas on how we should handle this member function then?
|
Hi, sorry for taking so long — this looks like a possibility, but we might have to change something. When the evaluation date changes, the helpers rebuild the swap in |
|
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. |
|
This PR was automatically closed because it has been stalled for two weeks with no further activity. |
(Partially) Closes #1393 .
This mr implements the idea from @eltoder to supply builders instead of adding more and more arguments to the constructors.
I will add some inline comments to places where I was contemplating different options myself. This hopefully simplifies the review process to some extent.