System.DayOfWeek vs NodaTime.DaysOfWeek

[Jon Skeet]

Hey folks,

We have an annoyance to resolve.

There's already a System.DayOfWeek enum, containing Sunday (0) to Saturday (6).

Joda has values in DateTimeConstants from Monday (1) to Sunday (7). These correspond with ISO-8601, and are currently in NodaTime.DaysOfWeek.

So, what should we do? We definitely shouldn't just leave the name as it is - it doesn't follow the naming conventions for enums, for a start (it's not a flags enum, so shouldn't be plural). We could decide to ignore ISO-8601 and just use System.DayOfWeek... or we could keep it, and rename it to something like IsoDayOfWeek. Personally I favour the latter approach - possibly with extension methods of ToSystemDayOfWeek (on IsoDayOfWeek) and ToIsoDayOfWeek (on System.DayOfWeek).

We can also make the properties which currently return an integer for the day of week (e.g. LocalDateTime.DayOfWeek) return an IsoDayOfWeek.

I'll make these changes now (except the extension methods, which can come when we build a whole bunch of conversions) and we can revert them later... but I'd like a little discussion :)

Jon

[The Configurator]

That's exactly what I thought (two enums with extension methods) when I heard of the discrepancy.

I kind of resent that neither of them has Sunday = 1 where it belongs though!

[The Configurator]

The LocalDateTime.DayOfWeek is, I assume, connected to a certain Calendar. What if it's using a calendar where the first day of the week is Sunday? Does it return 1 for Sunday or for Monday?

[Jon Skeet]

I think the idea is that "first day of the week" is independent of the DayOfWeek property value. In particular, it's not really tied to the calendar itself: when using Gregorian calendars, different people will use different representations: Christians may use Sunday as the first day of the week, whereas business will often use Monday.

This is mostly only relevant for WeekOfYear and WeekOfWeekYear, which are typically business concepts. I'm not sure offhand whether there are different ways you can request that.

I'm not sure what happens in calendars with more than 7 days in the week... are there any such calendars, does anyone know?

Jon

[The Configurator]

For early Christians, Sunday, as well as being the first day of the week, was also the spiritual eighth day, as it symbolised the new world created after Christ's resurrection.

Seriously though, The Hermetic Lunar WeekCalendar is one of many proposed reforms to the Gregorian Calendar. The lunation is divided into the four Moon Phases and has 6, 7, 8, or 9 days depending on the actual time difference between the full moon, First Quarter, new moon and Last Quarter.

There are plenty of other non-7-day weeks referenced in that article, but none seem to be in use today.

Thanks, wikipedia.

[Jon Skeet]

Hmm... that would suggest that in theory, we really should be returning an integer instead of an enum... but I think the pragmatic approach is to use the enum, and then specify that the value may not be one of those defined within the enum, if the calendar in question uses a different week system.

Jon

[James Keesey]

I thought that day of week was tied to a calendar and only made sense
within the definition of a calendar. I don't think we can have a
global enum/int value for days of week, they must be defined in a
calendar system.

James

[Jon Skeet]

I think in theory it is indeed tied to a calendar - but if 99.9% of uses involve a 7 day week with an obvious (and reliable) conversion between calendars, then it probably makes sense to have it as an enum.

If anyone fancies doing a bit of research about the number of weekdays on different calendars so we can make a better decision, that would be great :)

I'm not too unhappy about making non-Gregorian calendars "2nd class citizens" to some extent, so long as there isn't too much pain (and providing the gain for the majority is reasonably significant.)

Jon

[Dianbi]

I'd prefer to not have DayOfWeek in date-time entities at all.

Week and DayOfWeek are calendar concepts. Any decision we would take
does not respond to all possible situations.

.NET BCL designers made many "practical" design decisions, and as a
result we have too much unclear responsibilities in one class
System.DateTime. Work with such classes involves discussions: "use
this property, but only if you have that assumption, instead use
another property" etc.

I think it would be better to separate concerns. Connection between
LocalDateTime and calendar can be established via some extension
method in the future.

On Oct 11, 12:30 pm, Jon Skeet <sk...@pobox.com> wrote:
> I think in theory it is indeed tied to a calendar - but if 99.9% of uses
> involve a 7 day week with an obvious (and reliable) conversion between
> calendars, then it probably makes sense to have it as an enum.
>
> If anyone fancies doing a bit of research about the number of weekdays on
> different calendars so we can make a better decision, that would be great :)
>
> I'm not too unhappy about making non-Gregorian calendars "2nd class

> citizens" to *some* extent, so long as there isn't *too* much pain (and

> providing the gain for the majority is reasonably significant.)
>
> Jon
>

[Dru Sellers]

+1 for the original ideas.

-d

[Jon Skeet]

On 11 October 2010 18:49, Dianbi <dmitry....@gmail.com> wrote:

I'd prefer to not have DayOfWeek in date-time entities at all.

Week and DayOfWeek are calendar concepts. Any decision we would take
does not respond to all possible situations.

.NET BCL designers made many "practical" design decisions, and as a
result we have too much unclear responsibilities in one class
System.DateTime. Work with such classes involves discussions: "use
this property, but only if you have that assumption, instead use
another property" etc.

I think it would be better to separate concerns. Connection between
LocalDateTime and calendar can be established via some extension
method in the future.

LocalDateTime already has a connection to a calendar. You can't have a LocalDateTime without it knowing what calendar it's talking about. Without a calendar, you only have a LocalInstant (which is now internal).

I totally agree that DateTime's responsibilities are completely unclear, but I don't think we have that problem.

If you wanted to remove LocalDateTime's connection to a calendar, what would you expect to be able to do with it?

Jon

[Dianbi]

Oh, sorry. I should definitely look at the source code before
answering.

LocalDateTime by concept is just "Instant+Calendar". Let's leave it as
is,

My point was to not making any assumptions regarding what kind of
calendar is under LocalDateTime. We can't change the world and make
things less difficult than there are.

Moreover, i'd move all properties which are wrappers over calendar
fields to extensions, Now it makes sense to add some "practical"
extensions, for example together with extension property
public int DayOfWeek(this CalendarSystem calendar)
{
get { return
calendar.Fields.DayOfWeek.GetValue(localInstant); }
}

we can add

public ISODayOfWeek ISODayOfWeek
{
get { (ISODayOfWeek)return
calendar.Fields.DayOfWeek.GetValue(localInstant); }
}

or better method that will throw exception for calendars with
different meaning of "day of week".

Later, it will be possible to add other extension methods to return
something like HermeticLunarDayOfWeek, not touching original concept
of "localInstant+calendar"

Enumeration values and extesnions for each calendar should be in the
separate namespace, if user want to work with ISO, he should
intentionally use corresponded namespace, extensions and enumerations.

Another option: add extensions to calendar itself.

Then usage pattern would be
localDateTime.Calendar. IsoDayOfWeek (possibly wrong result, don't
like it)
or
localDateTime.Calendar.GetISODayOfWeek (with exception for wrong
calendar)
or
((ISOCalendar)localDateTime.Calendar).IsoDayOfWeek (with casting
exception for wrong calendar).

I don't know what is better, but definitely don't like any
assumptions.


On Oct 12, 11:16 am, Jon Skeet <sk...@pobox.com> wrote:
> LocalDateTime already *has *a connection to a calendar. You can't have a

[Jon Skeet]

On 12 October 2010 10:09, Dianbi <dmitry....@gmail.com> wrote:

Oh, sorry. I should definitely look at the source code before
answering.

LocalDateTime by concept is just "Instant+Calendar". Let's leave it as
is

Not quite - it's a local instant + calendar. But yes :)

 

My point was to not making any assumptions regarding what kind of
calendar is under LocalDateTime. We can't change the world and make
things less difficult than there are.

I think we can assume that more people will be using the Gregorian calendar (or something very similar) than any other calendar. So long as we don't make it particularly hard for non-Gregorian users, why not make it easier for the majority?

 

Moreover, i'd move all properties which are wrappers over calendar
fields to extensions, Now it makes sense to add some "practical"
extensions, for example together with extension property
       public int DayOfWeek(this CalendarSystem calendar)
       {
           get { return
calendar.Fields.DayOfWeek.GetValue(localInstant); }
       }

It's not clear what you're really doing here - where is localInstant coming from? If this is an extension method, it has to be static, so you need to provide the local instant.

 

we can add

       public ISODayOfWeek ISODayOfWeek
       {
           get { (ISODayOfWeek)return
calendar.Fields.DayOfWeek.GetValue(localInstant); }
       }

Again, what is this a property of? LocalDateTime in this case?

 

or better method that will throw exception for calendars with
different meaning of "day of week".

Later, it will be possible to add other extension methods to return
something like HermeticLunarDayOfWeek, not touching original concept
of "localInstant+calendar"

Currently, if we just return IsoDayOfWeek, then users of HermeticLunarDayOfWeek will just need to perform a cast. We can easily add an extension method - which isn't quite as convenient as a property, of course - to hide those casts.

I suggest that fewer than 1 in 1000 developers will want to use that calendar... so I don't want to make the other 999 developers suffer for the sake of that 1.

Enumeration values and extesnions for each calendar should be in the
separate namespace, if user want to work with ISO, he should
intentionally use corresponded namespace, extensions and enumerations.

Another option: add extensions to calendar itself.

Then usage pattern would be
localDateTime.Calendar. IsoDayOfWeek (possibly wrong result, don't
like it)

or
localDateTime.Calendar.GetISODayOfWeek (with exception for wrong
calendar)
or
((ISOCalendar)localDateTime.Calendar).IsoDayOfWeek (with casting
exception for wrong calendar).

I don't know what is better, but definitely don't like any
assumptions.

What you're proposing is much closer to the Joda Time approach: "Let's be flexible, and assume that people may want to create their own calendars, their own fields" etc. Newsflash: no-one does it, and the flexibility has a huge conceptual cost.

My new approach for Noda is much more about making things easy for the vast, vast majority of developers. We'll provide non-Gregorian calendars, and even make them pretty easy to use - it's easy to say, "Use this calendar." But I'm going to assume that all calendars have days, months, weeks, years... and I'm willing to make some aspects slightly harder to use for obscure calendars which don't have an easy translation of week days between the "native" form and ISO days.

I think that this biased approach will end up with a much simpler framework for most people - and I'd rather create something which makes a big difference to many people's lives than something that elegantly solves a problem no-one actually has.

Jon

[Steinar Dragsnes]

I really agree with Jon in this.

It is important to get something out there, the more developers using Noda, the more forward thrust this project will get.

Cheers,
Steinar.

[Jon Skeet]

Thinking about this further, I like the idea of an extension method instead:

public static IsoDayOfWeek GetIsoDayOfWeek(this LocalDateTime ldt)
{

    return (IsoDayOfWeek) ldt.DayOfWeek;
}

This would be much nicer if we had extension properties, but it is what it is.

One option would be to move the other way - keep the DayOfWeek property returning an IsoDayOfWeek, and change the extension method do something like this:

public static int GetNumericDayOfWeek(this LocalDateTime ldt)
{

    return (int) ldt.DayOfWeek;
}

Still thinking...

Jon

[Dianbi]

No problems.
Just wanted to make more intentionaly using calendar-specific
concepts.

As for the latter options, my vote for the first one.

public static IsoDayOfWeek GetIsoDayOfWeek(this LocalDateTime ldt)
{
return (IsoDayOfWeek) ldt.DayOfWeek;

}

because all other properties are integers(consistency)

On Oct 12, 2:10 pm, Jon Skeet <sk...@pobox.com> wrote:
> Thinking about this further, I like the idea of an extension method instead:
>
> public static IsoDayOfWeek GetIsoDayOfWeek(this LocalDateTime ldt)
> {
>     return (IsoDayOfWeek) ldt.DayOfWeek;
>
> }
>

> This would be *much* nicer if we had extension properties, but it is what it

[James Keesey]

To bring back The Configurator's problem, not everyone agrees what the
first day of the week is and we could handle this through variations
(sub-classes or instances with different settings) of the
GregorianCalendar class. To do this we would have to not hard-code the
days of week values but define them on the calendar. If I could ask
for a calendar that has Sunday as the first day of the week and then
pass it around I would be able to easily change it to one that uses
Monday as the first day of the week without having to recompile or
change the sorting code or anything. It would just work ;-).

I know that you want to have the "common" case handled simply but I
would argue that the "common" case that you think exists doesn't. The
basics of the Gregorian Calendar are probably the most common in usage
(the months) but the little details (days of week) are where all of
the problems with all of the other date/time packages occur.

Just my (insert small currency amount here (another big problem--
NodaCash?)) worth.

James

[Jon Skeet]

On 12 October 2010 16:38, James Keesey <james....@gmail.com> wrote:

To bring back The Configurator's problem, not everyone agrees what the
first day of the week is and we could handle this through variations
(sub-classes or instances with different settings) of the
GregorianCalendar class. To do this we would have to not hard-code the
days of week values but define them on the calendar. If I could ask
for a calendar that has Sunday as the first day of the week and then
pass it around I would be able to easily change it to one that uses
Monday as the first day of the week without having to recompile or
change the sorting code or anything. It would just work ;-).

Just to check, what properties are you expecting to change when you change the first day of the week? Would you expect to get back different values from the "day of week" property? I doubt that that would happen... it would be more likely to be changes to the week number etc.

 

I know that you want to have the "common" case handled simply but I
would argue that the "common" case that you think exists doesn't. The
basics of the Gregorian Calendar are probably the most common in usage
(the months) but the little details (days of week) are where all of
the problems with all of the other date/time packages occur.

Okay, acknowledged. Let's try to work out concrete use cases, and make those as easy possible.

Got to go, laptop dying.

 

[The Configurator]

I'm a bit surprised there's no GregorianDateTime or other *DateTimes. Then, the GregorianDateTime would have a property DayOfWeek which returns an enum { Sunday, Monday, ... }; the hypothetical HermaticLunarDateTime would return an integer from DayOfLunation, and so forth.

Then again, that's how Java's built-in Date does it if I'm not mistaken, and that's not a very good framework at all.

[James Keesey]

Specifically I was thinking that if I ask for a calendar where Sunday
is the first day of the week and then ask for the days of the week I
will get them in the order { Sunday, Monday, Tuesday, Wednesday,
Thursday, Friday, Saturday } and if I asked for a Monday start then
the order would be { Monday, Tuesday, Wednesday, Thursday, Friday,
Saturday, Sunday }. And if I compared the days using a week day
comparer they would be in that order. It would be easiest for any code
that uses the calendar to be able to assume that the days are always
in ascending order by int value. Having written calendar display code
I know what a pain it is to have to keep track of the fact that with
one setting the values are { 0, 1, 2, 3, 4, 5, 6 } and with another
setting the order is { 1, 2, 3, 4, 5, 6, 0}.

Besides as was previously mentioned some calendars may have more or
less than 7 days in a week. If we have hard coded enums then people
will use them and code will be harder to localize. But it you really
feel that enums are the way to go then I still say they should be a
part of the GregorianCalendar(System) object. At least then a
programmer will know that they are using calendar specific values.

James

[Jon Skeet]

On 13 October 2010 00:13, James Keesey <james....@gmail.com> wrote:

Specifically I was thinking that if I ask for a calendar where Sunday
is the first day of the week and then ask for the days of the week I
will get them in the order { Sunday, Monday, Tuesday, Wednesday,
Thursday, Friday, Saturday } and if I asked for a Monday start then
the order would be { Monday, Tuesday, Wednesday, Thursday, Friday,
Saturday, Sunday }. And if I compared the days using a week day
comparer they would be in that order. It would be easiest for any code
that uses the calendar to be able to assume that the days are always
in ascending order by int value. Having written calendar display code
I know what a pain it is to have to keep track of the fact that with
one setting the values are { 0, 1, 2, 3, 4, 5, 6 } and with another
setting the order is { 1, 2, 3, 4, 5, 6, 0}.

Right. That sounds like we need separate types for week day comparisons and some sort of iterator. I don't think we should be returning different values based on the calendar though - I think that could cause issues.

Besides as was previously mentioned some calendars may have more or
less than 7 days in a week. If we have hard coded enums then people
will use them and code will be harder to localize. But it you really
feel that enums are the way to go then I still say they should be a
part of the GregorianCalendar(System) object. At least then a
programmer will know that they are using calendar specific values.

Well, they're currently in a type named "IsoDayOfWeek" which suggests to me that they're calendar-specific... but maybe that's not enough.

Given the feedback here, I'm leaning towards either having two properties or have the normal properties return integers and an extension method for GetIsoDayOfWeek. Actually, aside from anything else we could also then have GetSystemDayOfWeek for those who wanted to use the System.DayOfWeek enum (e.g. for interop with other systems).

Am I right in saying that makes everyone reasonably happy? We can see how awkward it ends up when we write some more concrete use cases.

Jon

[Lasse Vågsæther Karlsen]

I would think that you have at least two week thingies in play here:

* A week concept, saying that "weeks go from sunday to saturday" or "weeks go from monday to sunday"

* A week instance, containing the 3rd week of 2010 according to a specific calendar

The first is more of a set of properties, sort of like CultureInfo.

The second is a fixed period of time.

--
Lasse Vågsæther Karlsen

[Jon Skeet]

2010/10/13 Lasse Vågsæther Karlsen <la...@vkarlsen.no>

I would think that you have at least two week thingies in play here:

* A week concept, saying that "weeks go from sunday to saturday" or "weeks go from monday to sunday"

* A week instance, containing the 3rd week of 2010 according to a specific calendar

The first is more of a set of properties, sort of like CultureInfo.

The second is a fixed period of time.

Right. Now currently a Gregorian calendar constructor takes the number of days to consider as being part of the first week of the year, but I don't believe the calendar itself determines the first day of the week. We should definitely consider that as a feature - and check whether Joda time supports it at all, for example.

The period part is interesting - we have the concept of an Interval which is between two instants, but I wonder whether we ought to have a similar concept for two LocalDateTime values in the same calendar (i.e. Calendar + LocalInstant + LocalInstant internally). It would be nice to be able to say "I want to know the period for 2009" or "June 2009" or "the third week of the year" for example. I suppose the original Partial concept could cover that... but maybe it's worth pulling out more. Let's work out how we might want to use it, then design the API around that, and then work out when to do it :)

Jon

[Lasse Vågsæther Karlsen]

I can say from experience that "period of time" is something that at least I (we) would find very usable.

Our work revolves around time periods in all flavors and colors, and we do plenty of operations involving periods. We have an extensive routine library to chop up, combine, find overlaps, etc. with such periods.

If you decide to create such a period, consider creating a common interface or base class that involves a from-to part (+ the calendar), so that "Week" is just a special kind of period. But of course, I don't have to tell you guys basic OOP :)

--
Lasse Vågsæther Karlsen

[Jon Skeet]

Okay, I'm reworking this again.

Current plan (for each of ZonedDateTime, LocalDateTime and LocalDate):

• Put DayOfWeek back to returning an integer; a calendar-specific representation of the day of the week
• Add an IsoDayOfWeek property of type IsoDayOfWeek which will throw an exception if the calendar doesn't use ISO days of the week

I think this is the most pragmatic approach - it won't give compile-time safety of course, but that would be very hard to achieve in a neat way, I think. It avoids using extension methods as a workaround for not having extension properties, and things like that.

I'm hoping to push this into the newapi branch tonight, before I start the rework on Period (which I've tried a couple of times, and I'm beginning to get a feel for). Let me know if you think it's a bad idea.

Jon