Consider deprecating the sub-date accessors on `DatePeriod`?

[kluever]

DatePeriod is defined as:

class DatePeriod(
        override val years: Int = 0,
        override val months: Int = 0,
        override val days: Int = 0
) : DateTimePeriod() {
    override val hours: Int get() = 0
    override val minutes: Int get() = 0
    override val seconds: Long get() = 0
    override val nanoseconds: Long get() = 0
}

It might be nice to document and deprecate the sub-date accessors, since calling them is almost wrong (they'll always return 0).

[dkhalanskyjb]

The idea here is that one can use DatePeriod everywhere where it's possible to use DateTimePeriod, but also in some other places. For example, it is correct to add DatePeriod to Instant, DateTimePeriod to Instant, DatePeriod to LocalDate, but not DateTimePeriod to LocalDate. So, when what you need is a DateTimePeriod, it doesn't matter if it's also a DatePeriod.

[kluever]

Does it ever make sense to directly call a time-based accessor on a statically typed DatePeriod though?

[dkhalanskyjb]

Ok, I thought you proposed that we get rid of these accessors altogether by removing the inheritance.

Even if we ignore the issue that hiding them would lead to weird problems where downcasting breaks the code, hiding functions is non-trivial with the current infrastructure. Marking them as deprecated would be inaccurate as we don't actually intend to remove them. We could force users to OptIn, but adding a new API marker for this seems like an overkill, given that it's unclear what actual harm could be done by using these accessors: since their result is static, they can't really do anything unexpected.

[kluever]

I'm not as familiar with deprecation rules/best practices in Kotlin, but in Java, deprecated just means "discouraged", not "this is going away".

We have some APIs that are "born deprecated" in an effort to discourage use, even if their use is harmless (e.g., Truth's StringSubject.isEquivalentAccordingToCompareTo​(String) is deprecated and discourage in favor of isEqualTo(String) because String comparison is always consistent with equality).

[dkhalanskyjb]

Yeah, in Kotlin, deprecation is mostly used to mean that something was superseded or is only left for legacy reasons. Though this would be stretching the definitions, we could "deprecate" datePeriod.hours in favor of just 0 if it turned out that these accessors are harmful, but as of now, the worst problem I can think of is that they slightly clutter the IDE completion on datePeriod..