Skip to content

Copy method #229

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

Closed
enciyo opened this issue Sep 15, 2022 · 9 comments
Closed

Copy method #229

enciyo opened this issue Sep 15, 2022 · 9 comments

Comments

@enciyo
Copy link

enciyo commented Sep 15, 2022

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
)
@dkhalanskyjb
Copy link
Collaborator

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: #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?

@enciyo
Copy link
Author

enciyo commented Sep 16, 2022

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
}

@dkhalanskyjb
Copy link
Collaborator

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.

@enciyo
Copy link
Author

enciyo commented Sep 19, 2022

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

@dkhalanskyjb
Copy link
Collaborator

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.

@enciyo
Copy link
Author

enciyo commented Sep 27, 2022

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.

@dkhalanskyjb
Copy link
Collaborator

dkhalanskyjb commented Sep 28, 2022

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.

@enciyo
Copy link
Author

enciyo commented Oct 3, 2022

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 enciyo closed this as completed Oct 11, 2022
@acmpo6ou
Copy link

acmpo6ou commented May 9, 2024

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants