-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Description
Environment
- Elixir 1.11.0-dev compared to earlier releases
Current behaviour in Elixir up to 1.10
In Elixir versions up to 1.10, the type t:Calendar.day_of_week has no @typedoc to clarify whether this is intended to refer to the cardinal day of the week (ie 1 == Monday, 2 == Tuesday) or whether it is intended to be the ordinal day of the week (ie 1 == 1st day of the week, 2 = 2nd day of the week). For all of these versions they are the same thing for Calendar.ISO since that calendar has the first day of the week to be Monday.
This is an issue for the Date.day_of_week/1 function because it states:
Returns the day of the week as an integer. For the ISO 8601 calendar (the default), it is an integer from 1 to 7, where 1 is Monday and 7 is Sunday.
Which would, I think, be interpreted most commonly as the cardinal use of the day_of_week.
New behaviour in Elixir 1.11
In Elixir 1.11, Calendar.day_of_week also has no @typedoc to clarify whether this is intended to refer to the cardinal day of the week or ordinal. However the result is now clearly an ordinal day of week as reflected in the updated docs for Date.day_of_week/1:
Calculates the day of the week of a given
date.
Returns the day of the week as an integer. For the ISO 8601
calendar (the default), it is an integer from 1 to 7, where
1 is Monday and 7 is Sunday.
Why is this an issue?
While the default behaviour for Calendar.ISO is unchanged, it's not possible to determine:
-
If other calendar implementations have treated
day_of_weekas a cardinal or ordinal number and the typet:Calendar.day_of_weekhas never been clear on the topic.Date.day_of_week/1today would tend to suggest cardinal. And as a side note, all my calendar implementations have followed this assumption (that however is not the motivation for this issue). -
If consumers of
Date.day_of_week/1have interpreted the result as a cardinal or an ordinal number and therefore may return an unexpected result in Elixir 1.11. This arises becauseDatefunctions are intended to return a semantically consistent result irrespective of calendar and today the semantics would appear, although not certain, to be a cardinal number.
Net: In the current versions of elixir the result is ambiguous but the same. In Elixir 1.11 the result is not ambiguous but may be different to Elixir 1.10 and earlier. And the documentation does not, I think, make that clear enough.
Some suggestions
-
Change the return type of
Date.day_of_week/1to bet:Calendar.ordinal_day_of_weekwhich would be added as a type ofnon_neg_integer. This at least clarifies the intent but does not remove the incompatibility between earlier releases and Elixir 1.11. Update the documentation is all areas to make clear which interpretation of day of week is being returned when. -
Leave the current Elixir 1.10 implementation of
Date.day_of_week/1in place and confirm its meaning to be a cardinal number. The typet:Calendar.day_of_weekstrictly means1==Monday, 2==Tuesdayand so on. Addt:Calendar.ordinal_day_of_weekandDate.ordinal_day_of_week/2to make clear it's returning thenthday of the week based upon the:starting_day.
My preference is (2) as you might expect since even though the current Date.day_of_week/1 returns an ambiguous result it is consistent for Calendar.ISO and doesn't disrupt the semantics of other calendar implementation. All calendars, including Calendar.ISO, can then add the new callback knowing explicitly when to return a cardinal day of week or an ordinal day of week.
Consumers are then also clear about when they are receiving a cardinal or ordinal number and they have a choice as to which one they want using either Date.day_of_week/1 to get the cardinal number or Date.ordinal_day_of_week/2 to get the ordinal number.