replace all Calendar with LocalDate (for java8)

[William1104]

By replacing Calendar with Java8's LocalDate, we don't need to worry about any DayLightSaving issue. Hope the change is useful.

[tonit]

Two things with this PR:

• 4,836 additions and 4,881 deletions.. Really? Looks like you changed (unintentionally?) EOL
• java.time.LocalDate is java8, finmath wants (?) to support down to java6 (at least thats what the pom says. You may correctly argue using Jodatime instead.

[cfries]

Hi.

I have two source branches. One is for Java 6, the other is for Java 8. Only issue is that some unit tests are for both branches and I like to have to code difference as small as possible (to ease adding features to both branches).

I will have a closer look a this PR later.

[William1104]

I also think the EOL may cause many unintentional changes. I will prepare another change to minimize the difference. I may first attach a gitattribute file to my fork version. I hope it will help my git work in a better way.

Regarding the java6 compatibility, may I include JodaTime for that source and test?

BTW, I am also using Eclipse. May I have your Java code formatting settings? I will help me to avoid changing any code unintentionally due to formatting shortcut.

[cfries]

Hi William.

Replacing Calendar on the src/java6 by the corresponding JodaTime class is a good idea.

For both changes (java8 (src/java) and java6 (src/java6)) I have a single concern: should we provide backward compatibility and also offer some methods keeping Calendar in its signature. On my side I have to check if there are applications relying on the Calendar API. I need to check that.

Apart from this the code chances look fine! (I had a look at your fork).

(Style file follows)

[cfries]

I have checked the code. Replacing Calendar by LocalDate (either Joda-Time (Java 6) or Java 8) is a good idea, but might break some compatibility. Adding methods with Calendar for backward compatibility is ugly and appears to be not worth it. I will switch to local date in the 2.x version (and will leave the 1.3.x version on a branch). OK?

[William1104]

Understood. When would be the 2.x version started?

[finmath]

This week. If it is OK for you, I will merge your change to the Java 8 branch manually (somehow git got confused, but the manual merge allows me to review the code, which I like to do anyway). Or do you have another pull request? I will then start to make the changes to the Java 6 branch.

Am 16.11.2015 um 01:50 schrieb William Wong notifications@github.com:

Understood. When would be the 2.x version started?

— Reply to this email directly or view it on GitHub https://github.com/finmath/finmath-lib/pull/6#issuecomment-156880032.

[William1104]

Thanks. I started to work on the java6 wth branch replace-cal-j6/ However, testcases suggested my changes was bad. I am still working on it.

[cfries]

Dear William.

I have already done that and pushed the changes. Unfortunately joda is slightly different from jdk 8. There should be a 2.0-SNAPSHOT in the repo now. 1.3.6 is on a branch.

I would be very happy if you could have a look.

Best C

Am 16.11.2015 um 16:32 schrieb William Wong notifications@github.com:

Thanks. I started to work on the java6 wth branch replace-cal-j6/ However, testcases suggested my changes was bad. I am still working on it.

— Reply to this email directly or view it on GitHub.

[cfries]

I did a review of this pull request. Since I had to review the changes and the EOL messed up my merge tool, I made a manual merge and pushed the changes to the Java 8 source. Thanks for proposing this and making the changes.

So far I only discovered a minor issue in TimeDiscreteEndOfMonthIndex - the code

        LocalDate endDate = referenceDate
                .plusDays((int)Math.round(evaluationTime * 365))
                .withDayOfMonth(1)
                .withMonth(fixingOffsetMonths+1);

should read

        LocalDate endDate = referenceDate
                .plusDays((int)Math.round(evaluationTime * 365))
                .withDayOfMonth(1)
                .plusMonths(fixingOffsetMonths);

(fixingOffset is just an offset, so it is "plus" and there was no "+1"-month-issue here (but there was in other places).

I also moved to LocalDate for the Java 6 source and pushed my changes to the repo as a 2.0-SNAPSHOT. I still have to test the code, feedback is welcome.

Note: At some places I would consider DateTime instead of a LocalDate, e.g. the Schedules. Another example is the evaluationTime. I assume that there are application where "time" might be relevant to allow for a valuation before payment and after payment (internally evaluationTime is a double and allow to consider "epsilon time-shifts").

[William1104]

Many thanks for accepting the proposed changes. Yes, I will take a look tonight or tomorrow. Hope my change works well and feel sorry for overlooking the month indexing on that function...

BTW, I have some other ideas about java8 enhancement. For example, implements a functor for random variable addition. It will make stream API work more naturally with those random variables.

[cfries]

I have found and fixed another small bug in the

Instead of

    endDate = endDate.withDayOfMonth(referenceDate.lengthOfMonth());

is should read

    endDate = endDate.withDayOfMonth(endDate.lengthOfMonth());

I am done with my tests. I have also performed some regression tests. I will release 2.0.0 with LocalDate tomorrow (given that you do not have any issue with the code).

With respect to Java 8 streams: The java 8 version of RandomVariable.java has an apply method and there is also a stream version implementing RandomVariableInterface - but stream support could be improved!