Kotlin / kotlinx-datetime

KotlinX multiplatform date/time library
Apache License 2.0
2.39k stars 99 forks source link

Feature: building instant from DateTimeComponents #364

Closed kunyavskiy closed 5 months ago

kunyavskiy commented 7 months ago

Whlile doc for DateTimeComponents says it's impossible to construct it on purpose, it looks like it is a convenient way of building Instant or maybe other primitives from it.

What do you think about add API like following

fun Instant.Companion.fromComponents(block: DateTimeComponents.() -> Unit) : Instant { 
  return DateTimeComponents().apply(block).toInstantUsingOffset()
}

Note, that one can already implement it with current public API, but it looks like a hack:

fun Instant.Companion.fromComponents(block: DateTimeComponents.() -> Unit) : Instant { 
  return DateTimeComponents.Format{}.parse("").apply(block).toInstantUsingOffset()
}
dkhalanskyjb commented 7 months ago

Please provide some code that you think is cumbersome to write without DateTimeComponents but is easy with it. Maybe we'll figure out a better way.

kunyavskiy commented 7 months ago

Basically, the example I had was building an instant from a date manually parsed by regex.

I've now found, that there is a constructor of LocalDate from components, which I missed and it mostly cover the problem. So probably, probably, the main usecase I have in mind is unfing code between formatting and different kotlinx.datetime class construction from some external format, where the only thing you need is an extention function filling DateTimeComponents.

But that doesn't sound so important.

dkhalanskyjb commented 6 months ago

unfing code between formatting and different kotlinx.datetime class construction from some external format, where the only thing you need is an extention function filling DateTimeComponents

So, the feature request is actually for something like this?

LocalDate.fromComponents { // receiver is LocalDateComponents
  year = someYear
  month = someMonth
  dayOfMonth = someDayOfMonth
}

This would be more in line with working with DateTimeComponents, but what are the benefits compared to just using the constructor? For DateTimeComponents, not having a constructor is a necessity, because the set of fields may increase in the future and the fields are optional, which is not the case for something like LocalDate. Having the compiler ensure that you provided all the necessary fields seems good.

a date manually parsed by regex

By the way, having to use regexes to parse dates is also probably a bug/missing feature in our library, so if it's us who made you go to regexes, please file another issue request.

kunyavskiy commented 6 months ago

It's not much useful for LocalDate, because it is basically subset of components.

While

Instant.fromComponents {
   year = someYear
   month = someMonth
   dayOfMonth = someDayOfMonth
   ...
}

makes more sense, as otherwise you would need exponential number of constructor in Instant (e.g. TimeZone vs UtcOffset, month vs monthNumber, year+month+dayOfMonth vs LocalDate).

dkhalanskyjb commented 6 months ago
LocalDateTime(someYear, someMonth, someDayOfMonth, someHour, someMinute).toInstant(TimeZone.of("Europe/Berlin"))
LocalDate(someYear, someMonth, someDayOfMonth).atTime(time).toInstant(UtcOffset.ZERO)

etc.

dkhalanskyjb commented 5 months ago

If there's still some interest in this, please let us know, and we'll reopen the issue.

kunyavskiy commented 5 months ago

Well, I argree that the options you suggested above already provides ability of doing what I have described. Although I still think mine are more convenient. But as it is pretty easy to implement it yourself, avoiding API duplication is fine.

dkhalanskyjb commented 5 months ago

It's not the API duplication that's the problem but making DateTimeComponents anything more than a thing for parsing and formatting. See, it's an unpleasant thing in general: it's mutable, non-thread-safe, people may be tempted to do invalid datetime arithmetic using its fields, etc. We'd rather not give DateTimeComponents any more power than it already has.