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

Add MarketTenor to represent ON/TN/SN/SW concept #2184

Merged
merged 3 commits into from Jun 26, 2020

Conversation

jodastephen
Copy link
Member

@jodastephen jodastephen commented Jun 25, 2020

Market tenors are distinct from tenors used in Strata

Market tenors are distinct from tenors used in Strata
Add market tenor support to FX Swap and Term Deposit conventions
* {@code Tenor} implements {@code TemporalAmount} allowing it to be directly added to a date:
* <pre>
* LocalDate later = baseDate.plus(tenor);
* </pre>
Copy link
Contributor

@brianweller89 brianweller89 Jun 26, 2020

Choose a reason for hiding this comment

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

Is this tenor reference leftover from a copy/paste?

* </ul>
* <p>
* The period from start date to end date is represented by {@link Tenor}.
* By contrast, the market tenor describes both the spot lag and tenor.
Copy link
Contributor

@brianweller89 brianweller89 Jun 26, 2020

Choose a reason for hiding this comment

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

It doesn't always describe the spot lag... for anything thats not 0 or 1 day it comes from the market convention.

Something like "By contrast, the market tenor describes the tenor and potentially a non-standard spot date"?

*
* @return true if this market tenor is short
*/
public boolean isShortSpotLag() {
Copy link
Contributor

@brianweller89 brianweller89 Jun 26, 2020

Choose a reason for hiding this comment

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

I'm not sure 'short' is the right term... for example GBP default spot lag is 0 days, so in that case the spot lag of 0 or 1 isn't short.

isNonStandardSpotLag()? It's a bit wordy but captures the concept more accurately

/**
* The order.
*/
private final int order;
Copy link
Contributor

@brianweller89 brianweller89 Jun 26, 2020

Choose a reason for hiding this comment

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

The order concept feels a bit too 'magic'... I get that it's trying to solve for the problem of some market tenors having a known number of days whilst others use the default spot lag but it takes some getting your head around.

Could use a private SpotType enum with values ZERO_DAYS, ONE_DAY and MARKET_STANDARD? You'd need a bit more code in adjustSpotLag to map to the right number of days for the non standard one.

Alternatively you could keep the order concept but associate it with the private enum values (or just use ordinal()). Might give you the best of both worlds in terms of making it easier to read and keeping the code in the methods simple

Copy link
Member Author

@jodastephen jodastephen Jun 26, 2020

Choose a reason for hiding this comment

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

An enum feels over the top, but I think a variable rename and a constant make it clearer.

@jodastephen jodastephen requested a review from brianweller89 Jun 26, 2020
@brianweller89 brianweller89 merged commit 7c30cb2 into master Jun 26, 2020
5 checks passed
@delete-merged-branch delete-merged-branch bot deleted the topic/tenor-code branch Jun 26, 2020
@jodastephen jodastephen added this to the v2.8 milestone Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants