Cleanup ZonedDateTime & OffsetDateTime Spec

[pipobscure]

Basic cleanup of the spec to closer match objects.md .

Changes

Eliminate OffsetDateTime.prototype.withZone

Reason is that it's just as easy to offsetdatetime.instant.withZone() which is much more explicit.

[ljharb]

I’m not sure it’s “just” as easy; it means you have to know to chain off the instant. It’d be ideal imo if everything had a withZone convenience method directly.

[pipobscure]

The question then is what happens if the offset doesn’t match? Is that an error or should it be just a convenience version for odt.instant.withZone()

how about the same thing on ZonedDateTime ? Should that be there? Should that do a timezone conversion?

I have no objection to keeping it, I just didn’t want to come up with a consistent rule here.

If a few people chime in, I’m happy with adding it back in (separate PR/will create an Issue when merging)

[gibson042]

The question then is what happens if the offset doesn’t match? Is that an error or should it be just a convenience version for odt.instant.withZone()

There are potentially at least two issues with OffsetDateTime.prototype.withZone:

1. Agreement: Should offsetDateTime.withZone("America/New_York") succeed if the offset cannot be preserved? The answer seems like it should be "no" for other with<Noun> methods, but those scenarios may be a little different (e.g., a nonexistent leap day like (new CivilMonthDay(2, 29)).withYear(2019) or a time of day lost to DST transition like CivilDateTime.fromString("2019-03-10T02:00").withZone("America/New_York")).
2. Priority: If we allow disagreement, should offsetDateTime.withZone("America/New_York") attempt to preserve the local date and time, or the instant? Either can yield a different offset, but the results will be different. Preserving the instant seems more logical (especially because it is the obvious choice if extended to zoned.withZone("America/New_York")), even though it is less consistent with the modeling of {Offset,Zone}DateTime–Instant as composition rather than conventional inheritance/refinement.

Having such a method only work when the new zone agrees with the existing object fields doesn't seem very useful to me because the window is so tight, but allowing disagreement seems likely to be confusing for the authors with different intuitions about our choice of data priority. I think I agree with @pdunkel that the method should be omitted, which means updating my mental model such that OffsetDateTime is not a hierarchical parent of ZonedDateTime, but rather more like a fraternal twin that exposes the same getters.

how about the same thing on ZonedDateTime ? Should that be there? Should that do a timezone conversion?

This is a separate question, essentially regarding reification of the class hierarchy. The primary purpose of a with<Noun> method is to return a new object of a more complete type in which the receiver provides data for overlapping fields and arguments provide data for non-overlapping fields. But if exposed on a descendant type that therefore already has those fields, it's essentially a shorthand for with (whose primary purpose is to return a new object of the same type in which all fields match the receiver except as overridden by the arguments), e.g. civilDate.withDay(1) vs. civilDate.with({ day: 1 }).

A complete reification would already be broken by including withZone on CivilDateTime but not OffsetDateTime, so that no longer seems particularly motivating to me. I would rather keep with<Noun> as purely augmentative and update with to support more fields (e.g., offsetDateTime.with({ offset: "+00:00" }) for offset conversion and zoned.with({ timeZone: "Asia/Tokyo" }) for time zone conversion).

[gilmoreorless]

should offsetDateTime.withZone("America/New_York") attempt to preserve the local date and time, or the instant?

I'm not overly familiar with JSR-310, but the explicit methods withZoneSameInstant and withZoneSameLocal (mentioned by @jodastephen and @pithu in #33 and #84) are a way to solve this exact dilemma. Although those methods are on ZonedDateTime, I also see similar methods on OffsetDateTime: atZoneSameInstant and atZoneSimilarLocal.

Regardless of the specific names (I won't rehash what @jodastephen has already said in #33), I think having 2 separate methods to remove the potential ambiguity is not a bad thing.

[pipobscure]

So we are back to a fight over names that make things less obvious.

I'd personally go with:
offsetDateTime.instant.withZone('America/New_York')

and

offsetDateTime.toCivilDateTime().withZone('America/New_York')

which makes it entirely obvious what to do and is no more complex than funcily named convenience methods that besides naming being less obvious since they pretend it's all about the zone rather than the base of what to combine with the zone.

[pipobscure]

I'll merge this for now. To get to a coherent base state to write more spec-text.