[Quantlib-dev] Latest commits

[Luigi B. <lui...@fa...>]

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