kotlinx-datetime icon indicating copy to clipboard operation
kotlinx-datetime copied to clipboard

Copy method

Open enciyo opened this issue 2 years ago • 4 comments

Hi, Sometimes, I need copy method like data class copy. I couldn't find any copy method. So, I created a new method for copy. But, I would expect this to be in the standard library.

I'm curious about your thoughts on this subject.

fun LocalTime.copy(
    hour: Int? = null,
    minute: Int? = null,
    second: Int? = null,
    nanoSeconds: Int? = null,
) = LocalTime(
    hour = hour ?: this.hour,
    minute = minute ?: this.minute,
    second = second ?: this.second,
    nanosecond = nanoSeconds ?: this.nanosecond
)

enciyo avatar Sep 15 '22 10:09 enciyo

This suffers from the same problem that led us not to provide LocalDateTime arithmetic: this function is oblivious to time zones. For example, on some days, in some places, 14:20:19 may just not exist because clocks are shifted from 14:00:00 to 15:00:00. Then, using LocalTime(13, 20, 19).copy(hour = 14) would create an day-time that is logically invalid.

We did get a request that may be similar though: https://github.com/Kotlin/kotlinx-datetime/issues/225, as well as several others. To design a good solution that is also safe, we are still collecting the use cases. Could you please describe yours?

dkhalanskyjb avatar Sep 16 '22 13:09 dkhalanskyjb

For my app, Seconds and nanoseconds are unimportant when comparing two localtime.

if(localTime.copy(second = 0, nanosecond = 0).compareTo(localTime2.copy(second = 0, nanosecond = 0)) == 0){
 // doSomething
}

enciyo avatar Sep 16 '22 21:09 enciyo

So, this is exactly the same request as #225—you just want truncation—just with an additional use case of comparing the two values.

Could you clarify the use case though? Why is it that 16:59:59.999999999 is equal to 16:59:00 (almost a minute off) but not to 17:00:00 (about exactly the same moment)? Without knowing about the specifics, it feels like a more natural comparison would be something like a (non-existent) localTime1.minus(localTime2) < 1.minute. This way, there would be no such discontinuity.

dkhalanskyjb avatar Sep 19 '22 10:09 dkhalanskyjb

I create 21 tasks to quit smoking. Each task has a time slot in it. For example, in the first task, I expect him to smoke at certain time intervals. I use time comparison to color the cards.

       val compareResult = today.time.copyWithResetSecond().compareTo(item.time.time.copyWithResetSecond())
       val color =
                if (compareResult == 0) MaterialTheme.colors.primary
                else if (compareResult < 0) MaterialTheme.colors.secondary
                else Color.Black

            Item(period = item, color = color)

https://github.com/enciyo/JetQuitSmoking/blob/master/app/src/main/java/com/enciyo/jetquitsmoking/ui/taskdetail/TaskDetail.kt#:~:text=val%20compareResult%20%3D,Color.Black

enciyo avatar Sep 19 '22 16:09 enciyo

Wouldn't the whole approach be buggy when DST transitions happen? For example, if the clocks are moved from 15:00 to 16:00 immediately, then having a colored card for the hour between 15:00 and 16:00 would be meaningless on that day.

dkhalanskyjb avatar Sep 27 '22 10:09 dkhalanskyjb

You're right. However, this project has not been completed yet. Also, clocks are not forwarded or backwards in my country. So if I publish the app in my country, it won't be a problem. But as I said, this project has not been completed yet and it is unclear in which countries it will be published.

I don't quite understand why DST transitions cause problems in the copy method. In my case I am able to accept the copy method oblivious to time zones.

enciyo avatar Sep 27 '22 20:09 enciyo

Can you be sure that a month from now your government doesn't decide that your country should move clocks forward and backward?

I don't quite understand why DST transitions cause problems in the copy method.

They cause problems because the copy method takes an existing LocalTime that likely makes sense in the context of a given day and time zone and creates a new LocalTime that is not guaranteed to make sense in that context. In the example above, LocalTime(12, 30, 0).copy(hour = 15) would create a meaningless LocalTime.

This is not a theoretical concern: we checked how people use LocalTime and LocalDateTime arithmetic and copy methods in Java and found great numbers of such bugs. So, we are not going to introduce something like copy unless we find a way to make it safe.

dkhalanskyjb avatar Sep 28 '22 10:09 dkhalanskyjb

I can understand your concern here. Created before DST LocalTime(12, 30, 0) . Even if we do nothing on this class, this class with DST time will be wrong for us. Am I right?

enciyo avatar Oct 03 '22 07:10 enciyo

In case you need such functionality, you can create an ext function yourself:

fun LocalDateTime.copy(
    year: Int? = null,
    monthNumber: Int? = null,
    dayOfMonth: Int? = null,
    hour: Int? = null,
    minute: Int? = null,
    second: Int? = null,
) = LocalDateTime(
    year ?: this.year,
    monthNumber ?: this.monthNumber,
    dayOfMonth ?: this.dayOfMonth,
    hour ?: this.hour,
    minute ?: this.minute,
    second ?: this.second,
)

then you can use it like this:

val dateTime = LocalDateTime(2024, 5, 8, 18, 25, 30)
dateTime.copy(hour=21) // equivalent to LocalDateTime(2024, 5, 8, 21, 25, 30)
dateTime.copy(year=2048) // equivalent to LocalDateTime(2048, 5, 8, 18, 25, 30)

acmpo6ou avatar May 09 '24 20:05 acmpo6ou