Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upDefault day count from index #1553
Conversation
jodastephen
added
the
Type:Enhancement
label
Sep 1, 2017
jodastephen
assigned
brianweller89
Sep 1, 2017
jodastephen
requested a review
from
brianweller89
Sep 1, 2017
brianweller89
requested changes
Sep 6, 2017
Few minor points on the review.
Can we add a test for the defaulting logic?
| FloatingRateIndex index = parseIndex(row, legPrefix); | ||
| if (index != null) { | ||
| // floating leg | ||
| indices.add(index); |
This comment has been minimized.
This comment has been minimized.
brianweller89
Sep 6, 2017
Contributor
indices.add(index) can be outside the if loop rather than here; will still add null for the fixed rate
| // fixed leg | ||
| indices.add(null); | ||
| if (!findValue(row, legPrefix, DAY_COUNT_FIELD).isPresent()) { | ||
| missingDayCount = true; |
This comment has been minimized.
This comment has been minimized.
brianweller89
Sep 6, 2017
Contributor
Do we even need the missingDayCount check? Feels like this is an unnecessary layer of complexity in what is already quite complex code.
What's the harm of passing a non-null default into the parseLegs method? Will just be ignored if the fixed leg has defined value.
This comment has been minimized.
This comment has been minimized.
jodastephen
Sep 11, 2017
Member
Case is where there is a fixed leg and two floating legs with different day counts. Must only trigger the "check day unique" logic when day count is actually missing.
| throw new IllegalArgumentException( | ||
| "Swap leg must define day count on fixed legs when more than 2 floating legs"); | ||
| } | ||
| defaultFixedLegDayCount = dayCounts.iterator().next(); |
This comment has been minimized.
This comment has been minimized.
brianweller89
Sep 6, 2017
Contributor
I like the readibility of Iterables.getOnlyElement() in these situations
| @@ -293,68 +377,49 @@ private static NotionalSchedule parseNotionalSchedule(CsvRow row, String leg) { | ||
| private static RateCalculation parseRateCalculation( | ||
| CsvRow row, | ||
| String leg, | ||
| FloatingRateIndex index, | ||
| DayCount defaultFixedLegDayCount, | ||
| BusinessDayAdjustment bda, | ||
| Frequency accrualFrequency, |
This comment has been minimized.
This comment has been minimized.
| if (frn.getType() == FloatingRateType.IBOR) { | ||
| // re-parse Ibor using tenor, which ensures tenor picked up from indexStr if present | ||
| Frequency freq = Frequency.parse(getValue(row, leg, FREQUENCY_FIELD)); | ||
| Tenor iborTenor = freq.isTerm() ? frn.getDefaultTenor() : Tenor.of(freq.getPeriod()); |
This comment has been minimized.
This comment has been minimized.
brianweller89
Sep 6, 2017
Contributor
Not immediately related to this PR but our getDefaultTenor() logic is not fully correct here.
Default tenor for most major currencies is 6M, with only USD being 3M
This comment has been minimized.
This comment has been minimized.
jodastephen
Sep 11, 2017
Member
* This is useful for providing a basic default where errors need to be avoided.
* The value returned is not intended to be based on market conventions."
jodastephen
added some commits
Aug 30, 2017
jodastephen commentedSep 1, 2017
Use additional
FloatingRateIndexlogic to default day count.Determines the index for each leg first, then calculates the default day count, then parses.