finmath / finmath-lib

Mathematical Finance Library: Algorithms and methodologies related to mathematical finance.
Apache License 2.0
488 stars 168 forks source link

New busDayCalendars and tests #31

Closed NiklasRodi closed 7 years ago

NiklasRodi commented 7 years ago

What is this PR for?

New busDayCalendars and tests:

What type of PR is it?

Improvement

cfries commented 7 years ago

This code does not work (at least not in all situations). You are checking if a date is a holiday via String comparison using LocalDate.toString(). The standard output LocalDate.toString() is ISO-8601 (2016-08-16) - see LocalDate. The list of dates used dd/MM/yyyy (16/08/2016). In addition: I would not rely on toString() having a specific format. It may depend on the users Locale settings.

I changed that by using a static initialised to a Set of LocalDates and using compare on them.

In addition the code has some performance issues. Scanning ArrayList is O(n). It is better to use a TreeSet here which is O(log(n)).

NiklasRodi commented 7 years ago

Error was introduced when converting my java6 code to Java8. It used to be org.joda.time.LocalDate.toString("dd/MM/yyyy"). Thanks for fixing that

cfries commented 7 years ago

Ah. I was wondering why the format could be different on your system.... now I see.