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

Provide construction and/or parsing for ZoneOffset #91

Closed
cromefire opened this issue Jan 3, 2021 · 6 comments
Closed

Provide construction and/or parsing for ZoneOffset #91

cromefire opened this issue Jan 3, 2021 · 6 comments
Labels
enhancement New feature or request
Milestone

Comments

@cromefire
Copy link

cromefire commented Jan 3, 2021

Currently you are not able to directly parse strings like +02:00 into an ZoneOffset. You can do so via

TimeZone.of("+02:00").offsetAt(Instant.fromEpochMilliseconds(0))

but it's rather a workaround, than a real solution (and if that's important probably less performant/efficient/discoverable).

My Idea would be to provide an ofHoursMinutes() (and maybe also ofHours(), ofHoursMinutesSeconds() and ofTotalSeconds​()) function similar to Java:

fun ofHoursMinutes(hours: Int, minutes: Int): ZoneOffset

and a parsing function that parses the time according to RFC3339 time-numoffset or time-offset (like Java's of()):

fun of(offsetId: String): ZoneOffset

ZoneOffset.of("+02:00")
ZoneOffset.of("Z")

A ZoneOffset.UTC would also be great.

This would greatly simplify #90.

edited to adjust my ideas more towards what Java already does

@cromefire
Copy link
Author

cromefire commented Jan 3, 2021

It's also probably doable in common code, so implementing it once should be sufficient (although for Java you could pretty much just delegate most things)

@cromefire cromefire changed the title Provide constructor and/or parsing for ZoneOffset Provide construction and/or parsing for ZoneOffset Jan 12, 2021
@ilya-g ilya-g added the enhancement New feature or request label Jan 17, 2021
@miguelhrocha
Copy link

Hey @cromefire do you still need someone to look into this? I can work on that this week

@cromefire
Copy link
Author

Yes I still think that's a good addition, because I mean if you can't parse a zone offset into a ZoneOffset that doesn't seem very intuitive, right?

@miguelhrocha
Copy link

I think it makes total sense, it'll also help Java devs wanting to adopt the library. I'll work on it 👍

@dkhalanskyjb
Copy link
Collaborator

Adding a way to parse ZoneOffset would not be a big change—more or less all the required code is in place, we only need to make it public under some name—but we plan to change the role that ZoneOffset plays in our library. Until then, we intentionally keep the API surface of ZoneOffset slim.

As a side note, with the latest release, it is also possible to parse a ZoneOffset like this:

TimeZone.of("+02:00") as ZoneOffset

@ilya-g
Copy link
Member

ilya-g commented Jul 2, 2021

In the course of ZoneOffset refactoring (see #125) we're introducing a new UtcOffset class instead of ZoneOffset which can be constructed in various ways:

  • parsed from an ISO string, e.g +02:00
  • from total number of seconds: UtcOffset(seconds = 1800)
  • from total number of minutes and number of seconds within a minute: UtcOffset(minutes = 90, seconds = 0)
  • from hours, minutes, seconds components: UtcOffset(hours = 3, minutes = 30)

@ilya-g ilya-g added this to the 0.3.0 milestone Jul 2, 2021
@ilya-g ilya-g closed this as completed Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants