Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for millisconds or fractional seconds to Instant.FromUtc(...) methods #142

Closed
GoogleCodeExporter opened this issue Mar 15, 2015 · 19 comments

Comments

@GoogleCodeExporter
Copy link

Please add support specifying milliseconds or fractional seconds to the 
Instant.FromUtc(...) methods

Original issue reported on code.google.com by mschuma...@giantspiral.com on 8 Nov 2012 at 3:40

@GoogleCodeExporter
Copy link
Author

Possibly. It would obviously be easy to do. But I don't want to encourage these 
methods to be used more than they should be - I was actually tempted to put 
them into NodaTime.Testing instead of the core package, because their primary 
use case should be for testing. Usually if you've got a collection of year, 
month etc values, it would be more appropriate to create a ZonedDateTime than 
an Instant. You can convert between them easily, but it just makes the intent 
clearer.

Original comment by jonathan.skeet on 8 Nov 2012 at 3:53

@GoogleCodeExporter
Copy link
Author

I've worked on multiple systems that use a combination of Java/Joda-Time and 
C#.  When dealing with timestamps the natural inclination is to use an Instant. 
 Since Noda-Time is based loosely on Joda-Time it would definately be nice if 
we didn't have jump through the "how many ticks are there in a millisecond?" 
hoop when moving data between Joda-Time systems and Noda-Time systems.  
ZonedDateTime doesn't support milliseconds either so that doesn't help much.

From going through the current Noda-Time API if I have have timestamp as 
millisconds since Unix Epoch I have to convert it to ticks before creating the 
Instant.  If the timestamp comes to me as a tuple of values I have to create an 
Instant for the non-subsecond components then create a Duration for the 
subsecond component then add the Duration to the Instant to get my final 
instant.

Original comment by mschuma...@giantspiral.com on 8 Nov 2012 at 4:24

@GoogleCodeExporter
Copy link
Author

Yes, if you get the value as milliseconds since the epoch, you have to convert 
to ticks first. That's very easy, and doesn't require you to know any values:

    Instant x = new Instant(millis * NodaConstants.TicksPerMillisecond);

If you're getting the data as a tuple of values, where did that data really 
come from? Usually it would have been parsed as a string - so get Noda Time's 
string parsing involved earlier. If you *really* have to have the values 
separately, construct a LocalDateTime and convert that to UTC.

Original comment by jonathan.skeet on 8 Nov 2012 at 5:43

@GoogleCodeExporter
Copy link
Author

(I'd be happier to add static FromMillisecondsSinceUnixEpoch and 
FromSecondsSinceUnixEpoch than extending FromUtc, btw. These are really two 
separate feature requests.)

Original comment by jonathan.skeet on 8 Nov 2012 at 5:45

  • Added labels: c

@GoogleCodeExporter
Copy link
Author

Original comment by jonathan.skeet on 8 Nov 2012 at 5:46

  • Removed labels: c

@GoogleCodeExporter
Copy link
Author

From*SinceUnixEpoch doesn't address this issue of how to create an Instant from 
a tuple of values that contains a subsecond component which is really my main 
issue.  Noda-Time can't do that unless I use a combination of Instant + 
Duration or create the timestamp using the BCL DateTime structure then creating 
my Instant from that. 

Original comment by mschuma...@giantspiral.com on 8 Nov 2012 at 7:00

@GoogleCodeExporter
Copy link
Author

> From*SinceUnixEpoch doesn't address this issue of how to create an Instant 
from a tuple of values that contains a subsecond component which is really my 
main issue.

This is what you get for putting two feature requests in one. "Milliseconds 
since an epoch" *is* a natural representation of an instant. There's no 
implicit time zone or calendar.

A tuple form is *not* a natural representation - because that's basically a 
LocalDateTime in a particular calendar system, which then needs to be 
interpreted in a time zone.

> Noda-Time can't do that unless I use a combination of Instant + Duration or 
create the timestamp using the BCL DateTime structure then creating my Instant 
from that.

No, you can use LocalDateTime, as I said before - which is appropriate IMO 
because it reflects what you've *actually* got: calendar-specific values. The 
reason there aren't constructors on ZonedDateTime taking particular parts is 
that the right way to think about ZonedDateTime is a LocalDateTime in a time 
zone, with an offset for differentiation in the ambiguous cases.

Original comment by jonathan.skeet on 8 Nov 2012 at 8:00

@GoogleCodeExporter
Copy link
Author

Noda-Time:
new ZonedDateTime(new 
LocalDateTime(year,month,day,hour,min,sec,milli,0),DateTimeZone.Utc,Offset.FromT
icks(0));

Joda-Time:
new DateTime(year,month,day,hour,min,sec,milli,DateTimeZone.UTC);

Are you saying that the Noda-Time is more correct, because it certainly isn't 
more concise as far as the API is concerned.


Original comment by mschuma...@giantspiral.com on 8 Nov 2012 at 8:59

@GoogleCodeExporter
Copy link
Author

No, I wouldn't suggest using that constructor in Noda Time. I'd use the 
"InZoneStrictly" method@

    new LocalDateTime(year, month, day, hour, min, sec, milli).InZoneStrictly(DateTimeZone.UTC);

Note how in Noda Time you're explicitly saying what you'd want to happen if the 
local time you were converting was ambiguous. It definitely won't be in UTC, 
but in other time zones it could. With Joda Time, there's no indication of what 
will happen.

There are *many* places where Joda Time has a huge number of overloads which 
Noda Time doesn't support - I've very deliberately tried to only provide the 
*absolutely logical* operations. Sometimes that will make code a little more 
verbose, but it will leave less room for error - and of course it provides a 
smaller API surface to learn.

Original comment by jonathan.skeet on 8 Nov 2012 at 9:04

@GoogleCodeExporter
Copy link
Author

"it won't be in UTC" -> "it won't be _ambiguous_ in UTC", I assume.  (The 
result _will_ be in UTC, of course, since that's what InZone*() does.)

Arguably LocalDateTime could have an InUtc() method in addition to the 
InZone*() methods, in the same way that Instant does, which would just do the 
above.  I _think_ that would be okay, although it doesn't strike me as useful 
as Instant.InUtc() is.

Original comment by malcolm.rowe on 8 Nov 2012 at 9:16

@GoogleCodeExporter
Copy link
Author

The "it won't be" was indeed meant to be paired with the "if it was ambiguous".

Yes, I'd be reasonably happy with LocalDateTime having an InUtc method. That 
doesn't violate any of the concepts, and could be convenient in a reasonable 
number of situations.

Original comment by jonathan.skeet on 8 Nov 2012 at 9:20

@GoogleCodeExporter
Copy link
Author

But in this case I know the time zone therefore its not a Partial but an 
Instant but to create the Instant I have to create a partial time first then 
convert it to an instant.  Semantically it feels wrong, to create an instant I 
have to create a partial first then convert it to an instant.

Original comment by mschuma...@giantspiral.com on 8 Nov 2012 at 10:19

@GoogleCodeExporter
Copy link
Author

"Partial" doesn't exist as a concept in Noda Time. Even in Joda Time, "partial" 
is about being able to hold some possibly-incomplete collection of fields, so 
it wouldn't really fit here. (LocalDateTime extends Partial in Joda Time, but 
you're really talking about local vs not local.)

You don't have an instant - you have bits which you need to *interpret* in a 
particular calendar and time zone in order to make them meaningful. It's the 
correct semantic approach - if you want to represent an instant in a purer way, 
use milliseconds since the epoch.

Whether this satisfies your needs or not, I'm going to add LocalDateTime.InUtc, 
Instant.FromMillisecondSinceUnixEpoch and Instant.FromTicksSinceUnixEpoch. (In 
the default 1.1 branch, of course.)

Original comment by jonathan.skeet on 8 Nov 2012 at 10:23

@GoogleCodeExporter
Copy link
Author

Revision 0ec3e1c9c02f adds the factory methods and conversions I'm happy with. 
You've still got a way to convince me to let you create an Instant directly 
from the bits :)

Original comment by jonathan.skeet on 8 Nov 2012 at 10:56

@GoogleCodeExporter
Copy link
Author

You already allow the creations on Instants from components, just not the 
complete set of components.  It seems kind of silly to draw the line in the 
sand at seconds when the precision of your instant allows for milliseconds and 
smaller.

Original comment by mschuma...@giantspiral.com on 9 Nov 2012 at 1:19

@GoogleCodeExporter
Copy link
Author

Yes, I allow creation of Instants from components *primarily for testing 
purposes*. I can't think of situations where it's the cleanest way to do things 
in production code, and now I'm wishing I really *had* kept this in the Testing 
package.

You never did answer why you're getting the components separately. I can 
imagine three ways you would receive the information:

- Via custom serialization, in which case the most appropriate form would be to 
use just ticks or millis since the epoch
- Via something which gives you a DateTime, in which case you can use 
FromDateTimeUtc
- Via a string, in which case you can use InstantPattern.

*Why* do you end up getting individual components for a kind of value which 
doesn't naturally *have* individual components?

Original comment by jonathan.skeet on 9 Nov 2012 at 6:46

@GoogleCodeExporter
Copy link
Author

Setting milestone to 1.1.0 because we've already done some work here, but note 
that we're still waiting for an answer to the above comment.

Original comment by malcolm.rowe on 24 Jan 2013 at 10:28

  • Changed state: NeedMoreInfo
  • Added labels: Type-Enhancement, Milestone-1.1.0

@GoogleCodeExporter
Copy link
Author

Original comment by malcolm.rowe on 3 Feb 2013 at 4:14

  • Added labels: Milestone-unscheduled
  • Removed labels: Milestone-1.1.0

@GoogleCodeExporter
Copy link
Author

NeedsMoreInfo for over a year -> closing.

Original comment by malcolm.rowe on 2 Aug 2014 at 8:59

  • Changed state: Invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants