Closed klin333 closed 2 years ago
I think I like the suggestion of the strings-as-enum codes as a layer of indirection. Let me think about that for a day, it is clearly done on other places / packages.
Parts of RQuantLib already uses string codes for day counters, eg https://github.com/eddelbuettel/rquantlib/blob/8e3d1ee2ec3470d9763a2e5dbe5a83bf9a48c744/R/bond.R#L522-L536
This strengthens the case for changing to string codes everywhere in RQuantLib. However, this would break a big proportion of existing user code. So maybe we check for the base type of user provided datecounter, then translate any numbers users give to string codes. The other way around also works, translate string codes to numbers.
I had a busy day so I didn't yet get to look at that but I think we can construct something backwards compatible (expecting int or numeric) yet allowing char a la (untested, riffing in the edit box here)
# standard function signature but we document that daycounter could be _int_ (or numeric) or _character_
if (is.character(daycounter)) <- daycounterStringToInt(daycounter)
# proceed as usual
Best of both worlds -- 'faster' direct ints but also 'safer', 'more expressive' textual values.
Whaddayathunk?
yep that looks good!
QuantLib day counter Thirty360 has various "conventions", all giving slightly different computations of Thirty360 day count, see https://rkapl123.github.io/QLAnnotatedSource/d5/d63/class_quant_lib_1_1_thirty360.html.
In RQuantLib src/utils.cpp, we already have many if else statements for the various conventions of ActualActual. It would be good to also allow RQuantLib users to choose the various conventions of Thirty360.
From a code structure point of view, probably not great extending the number of if else statements, because QuantLib appears to often add conventions. For example, current RQuantLib cases 8 to 13 are various ActualActual conventions. If we use let's say cases 14 to 19 for Thirty360 conventions, it would be rather ugly if in the future QuantLib adds another ActualActual convention. Would we make case 20 be the new ActualActual convention, so far away from the other ActualActual conventions? It would be hard to shuffle the numbers around without causing RQuantLib user pain. Maybe instead of using a number to denote day counters in RQuantLib, we need to move to using string codes...