Make DateTimeUnit constructors non public

[PaulWoitaschek]

I just got hit by an exception when adding days to a LocalDate

I took a look at the function signature:

public actual fun LocalDate.plus(unit: DateTimeUnit.DateBased): LocalDate = plusNumber(1, unit)

So I was using DateTimeUnit.DateBased to dynamically add days:

loopDate.plus(DateTimeUnit.DateBased.DayBased(fastingDays.size))

This was throwing an exception:

java.lang.IllegalArgumentException: Unit duration must be positive, but was 0 days.
	at kotlinx.datetime.DateTimeUnit$DateBased$DayBased.<init>(DateTimeUnit.kt:68)

On the slack channel I discovered that I should not use the DayBased constructor at all and instead use a period:

.plus(DatePeriod(days = 0))

I would suggest to mention that in the documentation and make the constructors internal so such usage errors don't happen.

[dkhalanskyjb]

Without answering the overall point you are making (for now!), I'll still need to point out that the general problem wouldn't be solved by just hiding the constructors, as nothing could prevent the programmer from writing DateTimeUnit.MONTH * fastingDays.size. So, hiding the constructors wouldn't prevent the error but would hide the functionality of having arbitrary units of time, as operators are not as discoverable as constructors. This doesn't seem like the best solution.

Also, for this specific purpose, you don't need a DatePeriod:

loopDate.plus(fastingDays.size, DateTimeUnit.DAY)

[ilya-g]

I'd rather deprecate the function plus taking DateTimeUnit alone implying value being added equal to 1. This way it would be more clear that DateTimeUnit is not a substitute for Duration or DateTimePeriod.

[dkhalanskyjb]

Another possibility could be to stretch the definition of measurement units and allow any number to be treated as a unit. As far as I can tell, the only problem then would be with until(DateTimeUnit), which could conceivably check for zero-sized units and throw in such cases.