[m-rev.] for review: Java implementation of time library now tested and correct

[Fergus Henderson]

On 16-Dec-2003, James Goddard <goddardjames at yahoo.com> wrote:
> Implemented some library functions for the time library in java.

Next time, if you submit a new version of a change after some review
comments on an earlier version, could you please post a relative diff
that shows what has changed this time around, as well as the usual
full diff?

You can produce relative diffs either by using "interdiff"
or by saving a copy of your files each time and then just using diff.

> @@ -375,6 +380,18 @@
> +:- pragma foreign_proc("Java",
> +	time__c_time(Ret::out, _IO0::di, _IO::uo),
> +	[will_not_call_mercury, promise_pure, tabled_for_io],
> +"
> +	// Milliseconds should be discarded so that
> +	//	mktime(localtime(Time)) == Time
> +
> +	java.util.GregorianCalendar gc = new java.util.GregorianCalendar();
> +	gc.set(java.util.Calendar.MILLISECOND, 0);
> +
> +	Ret = gc.getTime();
> +").

Hmm... I'm not sure that discarding the milliseconds is the right thing
to do.  It might be better to keep them, and change the test case.

For code using the foreign language interface, discarding the milliseconds
isn't necessarily enough to guarantee that mktime(localtime(Time)) = Time,
because users might use the foreign language interface to create times
that don't have their milliseconds zeroed.

So yes, please keep the milliseconds, and change the test case.

> @@ -598,6 +722,39 @@
>  		new System.DateTime(Yr + 1900, Mnt + 1, MD, Hrs, Min, Sec);
>  	Time = local_time.ToUniversalTime();
>  }").
> +:- pragma foreign_proc("Java",
> +	time__c_mktime(Yr::in, Mnt::in, MD::in, Hrs::in, Min::in, Sec::in,
> +		_YD::in, _WD::in, N::in, Time::out),
> +	[will_not_call_mercury, promise_pure],
> +"
> +	java.util.GregorianCalendar gc = new java.util.GregorianCalendar(
> +			Yr + 1900, Mnt, MD, Hrs, Min, Sec);
> +
> +	Time = gc.getTime();
> +
> +	// Correct for DST:  This is only an issue for 2 hours of the year.
> +	// (In Melbourne, 2:00am-2:59am occur twice when leaving DST)
> +
> +	// If the time we constructed is not in daylight savings time, but
> +	// it should be, we need to subtract 1 hour.
> +	if (N == 1 && gc.getTimeZone().inDaylightTime(Time) == false) {
> +		Time.setTime(Time.getTime() - 3600000);
> +		if (gc.getTimeZone().inDaylightTime(Time) == false) {
> +			throw new RuntimeException(
> +				""time__mktime: failed to correct for DST"");
> +		}
> +	}
> +
> +	// If the time we constructed is in daylight savings time, but
> +	// should not be, we need to add 1 hour.
> +	if (N == 0 && gc.getTimeZone().inDaylightTime(Time) == true) {
> +		Time.setTime(Time.getTime() + 3600000);
> +		if (gc.getTimeZone().inDaylightTime(Time) == true) {
> +			throw new RuntimeException(
> +				""time__mktime: failed to correct for DST"");
> +		}
> +	}
> +").

Hard-coding 3600000 (1 hour) is not correct here.  Some time zones have
different daylight savings time offsets.

What happened to the code using DST_OFFSET?

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------