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 time only representation, e.g. LocalTime #57

Closed
molikuner opened this issue Oct 8, 2020 · 18 comments · Fixed by #187
Closed

Provide time only representation, e.g. LocalTime #57

molikuner opened this issue Oct 8, 2020 · 18 comments · Fixed by #187
Labels
enhancement New feature or request
Milestone

Comments

@molikuner
Copy link

Hey,

Currently there is support for LocalDateTime, but I would like to use some time representation without a date. Ideall would be something like LocalTime for my use case.

my use case:
represent daily events

my current workaround:
use DateTimePeriod with a max of 24h in total

@kluever
Copy link

kluever commented Oct 8, 2020

(CC'ing myself on this issue)

@Jolanrensen
Copy link

+1 for me. Having a simple object representing a TimeOfDay would be really helpful in a library. It could even encompass AM/PM if our American friends would like that.

@christian-draeger
Copy link

+1

@ilya-g ilya-g changed the title support time only representation Provide time only representation, e.g. LocalTime Jan 17, 2021
@ilya-g ilya-g added the enhancement New feature or request label Jan 17, 2021
@dkhalanskyjb
Copy link
Collaborator

dkhalanskyjb commented Mar 16, 2021

After some thought, it does look like there is a real need for a data structure to represent time-of-day wall clock readings, so we are interested in adding it!

The next step is to gather more details about the use cases: which particular operations are expected from such a data structure?

For now, the following ones come to mind:

  • (De)serialization is the obvious one;
  • Combination with LocalDate to form a LocalDateTime;
  • Arithmetic that wraps at midnight (seems error-prone, as this would ignore zone offset transitions, but maybe someone could make a compelling argument for why manipulating wall-clock time directly is needed?)

Anything else?

@xsveda
Copy link

xsveda commented Mar 16, 2021

We are currently using our own implementation of LocalTime for purposes of presenting day to day restaurant menu and possibility to place a table reservation. The API of our class is:

data class LocalTime(val hour: Int, val minute: Int, val second: Int = 0, val nanosecond: Int = 0) : Comparable<LocalTime>

val LocalTime.Companion.noon: LocalTime
val LocalTime.Companion.midnight: LocalTime
fun LocalTime.Companion.parse(iso: String): LocalTime
operator fun LocalTime.plus(duration: Duration): LocalTime

val LocalDateTime.time: LocalTime
fun LocalDateTime.Companion.of(date: LocalDate, time: LocalTime): LocalDateTime

Also were similar implementation is available under this project by @fluidsonic:
https://github.com/fluidsonic/fluid-time/blob/main/sources/LocalTime.kt

@dkhalanskyjb
Copy link
Collaborator

Thanks @xsveda! Could you please describe what LocalTime.plus does when the result exceeds 24 hours and why you need such a method?

@xsveda
Copy link

xsveda commented Mar 16, 2021

Sure, it is an Android app and we hate the system time picker so we allow user to pick the start and the end time only from pre-generated options (by 30 minutes) that are presented like checkable buttons:
fun halfHourTimestamps(from: LocalTime, to: LocalTime) = generateSequence(from) { it + 30.minutes }.takeWhile { it <= to }

When it goes over 24h it resets so LocalTime(hour = 23, minute = 0) + 1.hours == LocalTime.midnight // == 00:00

@hrach
Copy link

hrach commented Mar 16, 2021

Last time I used (Java's) LocalTime for modeling Reminder entity, where user choose the time of the day they want to be repeatadly reminded.

  • (de)serialization - manually to hour & minute property (rather a backend decission)
  • combination with local date & time - no afaik, we created only the resulting ZonedDateTime like
    return reminder.daysOfWeek.map { dayOfWeek ->
              ZonedDateTime.now()
                  .withDayOfWeek(dayOfWeek)
                  .withHour(reminder.time.hour)
                  .withMinute(reminder.time.minute)
                  .withSecond(0)
                  .withNano(0)
          }
  • no arithmetic; I would expect the one described by xsveda.

@fluidsonic
Copy link

  • I have a lot of someLocalDate.atTime(LocalTime.midnight) (or other local times).
  • I use it for basic formatting (custom LocalTime.format() extension function).
  • I store in a database… a local time. It's part of my data model, e.g. in an event management system (what time happens what on a given day).

That's basically it.

@dkhalanskyjb
Copy link
Collaborator

@xsveda here is some alternative code for the use case you provided. Would you say that it reflects the intent better or worse than what you've presented?

    fun halfHourTimestamps(atDate: LocalDate, from: LocalTime, to: LocalTime, zone: TimeZone) =
        generateSequence(LocalDateTime(atDate, from).toInstant(zone)) {
            it + 30.minutes
        }.map {
            it.toLocalDateTime(zone).time
        }.takeWhile {
            it <= to
        }

The crucial difference is that this code takes daylight saving time transitions into account, which will rarely cause some instances of LocalTime to be omitted, and some repeated in the resulting sequence.

For example, clocks were shifted an hour back at 2008-11-02T02:00 in New York, so the following behavior is observed:

halfHourTimestamps(
    LocalDate(2008, 11, 2),
    LocalTime.parse("01:00"),
    LocalTime.parse("10:00"),
    TimeZone.of("America/New_York"))
// [01:00, 01:30, 01:00, 01:30, 02:00, 02:30, 03:00, 03:30, 04:00, 04:30, 05:00, 05:30, 06:00, 06:30, 07:00, 07:30, 08:00, 08:30, 09:00, 09:30, 10:00]

(note that both 01:00 and 01:30 are repeated twice, which reflects the fact that an hour after 01:00 it was 01:00 in New York that night)

@xsveda
Copy link

xsveda commented Mar 17, 2021

Interesting idea, I didn't think about it this way. I believe it may work for some use cases but not sure whether it would prefer it for ours.
In our use case we generate table reservation slots based on restaurant opening hours that are stable for all weeks of the year. Of course, that every reservation has a date but currently we do not necessarily need to take the restaurant's time zone into account.
We might adopt your suggestions, but the code would be more complex due to:

  1. introduction of time zones that need to be defined by restaurant owner of inferred from restaurant address/position
  2. filtering of possible duplicities when time is shifted back as those do not make sense for the customer

On the other hand, for restaurant opened 24/7 it will generate correct time slots in the autumn when there is no 2 AM slot as time is shifted forward :)

@dkhalanskyjb
Copy link
Collaborator

dkhalanskyjb commented Mar 18, 2021

introduction of time zones that need to be defined by restaurant owner of inferred from restaurant address/position

Does not seem all that terrible. For any fixed-offset time zones, the proposed solution works the same as the one you provided, so having a default of TimeZone.UTC or something in the same vein would mean that only the owners who don't mind manually setting their time zones would be affected.

filtering of possible duplicities when time is shifted back as those do not make sense for the customer

I would argue that the duplicities should, in fact, not be filtered. If the time shift occurred outside of working hours, it won't be reflected in the schedule anyway, but if it occurred during the workday, then some time slots would have an ambiguous meaning. So, a user who made a reservation at 13:00 on a day when 13:00 occurs twice wouldn't know when the reservation actually begins and so would have to call the restaurant or do something else like that. If, however, duplicate entries were present, the user could conceivably remember (or be reminded by some text in the app) that there is a DST transition; then they would explicitly be making a reservation at the first time it's 13:00.

The bottom line is that it looks like the domain you're modelling does require handling time zone transitions, you just simplified this for the sake of ease of use in exchange for robustness. This may be a perfectly valid tradeoff in your case, but, from the library design perspective, the choice to ignore transitions must be done explicitly and with the full understanding of the consequences, so it seems for now that the (problematic in many cases) LocalTime arithmetic should not be very easily available.

Thanks for the elaboration, this does help a lot!

@PaulWoitaschek
Copy link

We have a lot of cases where we need a LocalTime.
Currently we are rolling our own version of LocalTime.

For example we have a larger fasting feature where every fasting day has different fasting times which don't have a date:

internal data class FastingTime(
  val start: LocalTime,
  val end: LocalTime,
  val changed: Boolean
)

Based on that time we derive additional logic like for example:

  private val FastingTime.isFullDayFasting: Boolean
    get() {
      return start == LocalTimeMin && end == LocalTimeMax
    }

@vanniktech
Copy link
Contributor

I've needed some this too (Android + JVM)

  • basic LocalTime data class (hour, minutes, second + nano)
  • val LocalDateTime.time extension property
  • LocalDate.atTime(localTime: LocalTime) helper
  • LocalTime#toSecondOfDay
  • LocalTime#toNanoOfDay
  • LocalTime#ofSecondOfDay
  • LocalTime#ofNanoOfDay

This is what I have in commonMain:

import kotlinx.datetime.LocalDate
import kotlinx.datetime.LocalDateTime
import kotlinx.datetime.atTime

private const val MINUTES_PER_HOUR = 60
private const val SECONDS_PER_MINUTE = 60
private const val SECONDS_PER_HOUR = SECONDS_PER_MINUTE * MINUTES_PER_HOUR

private const val NANOS_PER_SECOND = 1000_000_000L
private const val NANOS_PER_MINUTE = NANOS_PER_SECOND * SECONDS_PER_MINUTE
private const val NANOS_PER_HOUR = NANOS_PER_MINUTE * MINUTES_PER_HOUR

data class LocalTime(
  val hour: Int,
  val minute: Int,
  val second: Int = 0,
  val nanosecond: Int = 0,
) {
  fun toSecondOfDay(): Int {
    var total = hour * SECONDS_PER_HOUR
    total += minute * SECONDS_PER_MINUTE
    total += second
    return total
  }

  fun toNanoOfDay(): Long {
    var total: Long = hour * NANOS_PER_HOUR
    total += minute * NANOS_PER_MINUTE
    total += second * NANOS_PER_SECOND
    total += nanosecond.toLong()
    return total
  }

  companion object {
    fun ofSecondOfDay(secondOfDay: Int): LocalTime {
      var seconds = secondOfDay
      val hours = (seconds / SECONDS_PER_HOUR)
      seconds -= (hours * SECONDS_PER_HOUR)
      val minutes = (seconds / SECONDS_PER_MINUTE)
      seconds -= (minutes * SECONDS_PER_MINUTE)

      return LocalTime(
        hour = hours,
        minute = minutes,
        second = seconds,
      )
    }

    fun ofNanoOfDay(nanoOfDay: Long): LocalTime {
      var nanos = nanoOfDay
      val hours = (nanos / NANOS_PER_HOUR)
      nanos -= hours * NANOS_PER_HOUR
      val minutes = (nanos / NANOS_PER_MINUTE)
      nanos -= minutes * NANOS_PER_MINUTE
      val seconds = (nanos / NANOS_PER_SECOND)
      nanos -= seconds * NANOS_PER_SECOND

      return LocalTime(
        hour = hours.toInt(),
        minute = minutes.toInt(),
        second = seconds.toInt(),
        nanosecond = nanos.toInt(),
      )
    }
  }
}

val LocalDateTime.time get() = LocalTime(
  hour = hour,
  minute = minute,
  second = second,
  nanosecond = nanosecond,
)

fun LocalDate.atTime(localTime: LocalTime) = atTime(
  hour = localTime.hour,
  minute = localTime.minute,
  second = localTime.second,
  nanosecond = localTime.nanosecond,
)

and for Android/JVM I also have:

fun java.time.LocalTime.toKotlinLocalTime() = LocalTime(
  hour = hour,
  minute = minute,
  second = second,
  nanosecond = nano,
)

fun LocalTime.toJavaLocalTime() = java.time.LocalTime.of(
  hour,
  minute,
  second,
  nanosecond,
)

Would this already be something that is helpful to others and if so can be pr'ed?

I haven't worked out serialisation adapters as I'm only saving them to the database where I use secondOfDay/nanoOfDay. This could be on top, as well as other needs.

@bishiboosh
Copy link
Contributor

This is also needed for date-time handling in https://github.com/akuleshov7/ktoml so @vanniktech did you make a PR (with serialization adapters) ?

@vanniktech
Copy link
Contributor

I did not since you're the first to respond. Personally, I don't need serialization adapters. I store my values as a Long inside my Database using either nano of day / second of day representation (depending on the precision I need), so that I can get free long sorting.

If you have time, feel free to take my snippets and convert it into a PR, otherwise I can get back to it later this or next week.

@bishiboosh
Copy link
Contributor

I'm gonna see if I get some time to do it :)

@bishiboosh
Copy link
Contributor

I made a PR, @kluever @dkhalanskyjb could you maybe review it ?

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

Successfully merging a pull request may close this issue.