Latest commits
Posted by Luigi Ballabio-2 on Jul 22, 2003; 8:29am
URL: http://quantlib.414.s1.nabble.com/Latest-commits-tp10269.html
Andre,
first things first: it's a great job you did on date calculations and all
the other stuff, so kudos to you.
Secondly, I'm going to nag you just a tiny bit---feel free to come after me
with a big clown-sized rubber hammer...
In random order:
a) Calendar::isLastBusinessDayOfMonth seems quite a lot of writing
(although I'm sure I came up with names just as long in the past, so I
ain't throwing the first stone here). How about Calendar::isEndOfMonth?
b) As a matter of personal taste entirely, and therefore fully discardable
on your part, I'm not enthusiastic about Date::Date(string). I'd rather
keep the class clear from IO (Ditto for Period.) But I can live with it.
(Aside: if the constructor was prompted by:
string s1, s2;
cin >> s1 >> s2;
Date d1(s1,fmt);
Date d2(s2,fmt);
how about we implement operator>> and an IO manipulator instead, so that
one can write:
cin >> setdateformat(fmt);
Date d1, d2;
cin >> d1 >> d2;
In the other cases, writing "Date d = DateParser::parse(s,fmt)" hurts me
less, but that's only me :) end of aside)
c) Maybe Period::operator== should be a partial function. There's a subset
of the domain where it is well defined ("1 year == 12 months" is ok, and so
is "3 days == 3 days" and "14 days == 2 weeks") but how about "31 days == 1
month"? Or "4 weeks == 1 month"? With the current implementation, the first
is false, with the disconcerting result that if d = Date(22,7,2003), p1 !=
p2 but d.plus(p1) == d.plus(p2). The second is true, but for most dates,
d.plus(p1) != d.plus(p2). How about throwing an exception if the == is
undecidable?
d) Date::ascending() is none other than std::less<Date>(). In the same way,
Date::descending() is std::greater<Date>() and Date::less_equal(d) is
std::bind2nd(std::less_equal<Date>(),d). If those classes are to be used
with the STL, probably we better use their STL implementation :) Especially
Date::less_equal might be more confusing than useful, since std::less_equal
is a binary function. And in general, when one familiar with the STL reads
std::less<Date>, he knows what that it. If he reads Date::ascending, he
doesn't know. The same goes for TimeBasket::Entry.
e) Scheduler. No problem per se---on the contrary, I'm grateful you tackled
the thing---but it starts to have too many optional parameters. We could
make it a bit prettier with the named parameter idiom, i.e.,
Scheduler s1 = MakeScheduler(mandatory parameters).withStubDate(d);
Scheduler s2 = MakeScheduler(mandatory parameters).backwards();
Scheduler s3 =
MakeScheduler(mandatory parameters).withStubDate(d).backwards();
f) CashFlowVectors---a lot of parameters now. Maybe we should pass them a
Scheduler? The same goes for SimpleSwap.
g) Swap::fairRate seems to work only if the first leg is the fixed one---I
wouldn't count on that. Also, Swap is a generic swap class: one could be
swapping two floating legs, so that fairRate() might not make sense at all.
Thanks again,
Luigi