ThreeTen / threeten

This project was the home of code used to develop a modern date and time library for JDK8. Development has moved to OpenJDK and a separate backport project, threetenbp.
http://threeten.github.io/
191 stars 37 forks source link

Determine the rightful nature of the Chronology Implementation #48

Closed RichardWarburton closed 11 years ago

RichardWarburton commented 12 years ago

At the moment there is a deliberate separation between the Chrono side of things and other parts of the API. Should this remain the case?

RogerRiggs commented 12 years ago

The requirement is that the national calendars must have the same level of support as currently provided to ISO including date+offset, date+time, date+time+offset, date+time+zone; plus Month objects that can hold any month; Era that is extensible as needed (for Japanese calendar).

One way to accomplish the major part of this is to make LocalDate extend ChronoDate and change the composites to hold a ChronoDate. If preference is given to ISO it should be by specialization not with a separate API.

Having two different models for calendars adds confusion to the API especially when there are two ISO Calendars with different behaviors.

Calendar APIs from Microsoft .NET Globalization, Windows NLS, IBM Components for Unicode (Java and C++) and Oracle E-Business Suite and Oracle Globalization Development Kit provide a more consistent API usable for all calendars.

jodastephen commented 12 years ago

The IBM classes are simply a continuation of java.util.Calendar, something I and most Java developers consider as a design to be avoided.

The .NET Globalisation framework says:

Therefore, when you use the methods provided by the DateTime structure, you must be aware that the members such as the DateTime.Day property, the DateTime.Month property, the DateTime.Year property, and the DateTime.AddDays method are based on the Gregorian calendar. Even if you change the current calendar in your application's code or change date and time settings through the Regional and Language Options Control Panel, the Gregorian calendar is still used to perform the calculations for these methods. This functionality prevents the arithmetic performed by these methods from being corrupted by a user's settings.

This is exactly the same as LocalDate/LocalDateTime etc. Microsoft are making the same decision I am to isolate the common ISO-8601 use case.

It continues

If you want to perform culture-sensitive date and time operations based on the current calendar, you must use the DateTimeFormatInfo.Calendar property to call methods provided by the Calendar class such as Calendar.GetDayOfMonth, Calendar.GetMonth, Calendar.GetYear, and Calendar.AddDays.

The Calendar API, translated to Java, is as follows:

localDateTime= chrono.addYears(localDateTime, 5)
year = chrono.getYear(localDateTime)

I would argue the .NET example strengthens the case for separating Chrono code from ISO code. I can't say that I love the non-OO nature of the .NET calendar class, but it is effective at isolating the calendar system code from normal/real code.

I'll discuss the issues with making LocalDate extend ChronoDate another time.

jodastephen commented 12 years ago

A full discussion of the options has been written up.

RogerRiggs commented 12 years ago

The issue topic is poorly chosen and does not reflect the real issue as noted in my first comment. The issue is inadequate support for national calendars.

The criteria mentioned in this discussion do not reflect all of the requirements and the priorities among requirements.

A+) It should be easy to write a fully functional, type-safe library that is generic across all calendar systems, including ISO. For example, to schedule appointments or customer visits. The simple functions such as next month/previous month, day-of-week, and localization must be well supported. The API must be complete and a good base to build applications and other libraries.

Every Oracle product must be internationalized, with some calendar uses being configurable and some fixed. From experience, many developers take the easy path and use the API and defaults provided in the API. In many cases, that has required expensive and time consuming rework of the implementation when the defaults were no longer appropriate. It might be the case that best practice is to recommend only using the API that supports configurable calendars.

For the developers that must use the national calendar APIs they need to be designed to minimize the risk of misuse. This may suggest a more strongly typed interface rather than less. For example, an abstraction for months could eliminate possible misuse of month numbers and be able to verify that the months of different calendars are not mixed.

The same abstractions are the ISO API and ChronoDate API but they are realized differently, that asymmetry is a source of possible confusion and puts a larger burden on developers. It also raises the cost of maintaining a large body of code.

RichardWarburton commented 12 years ago

I've renamed the issue in order to reflect the broader nature of the discussion.

RogerRiggs commented 12 years ago

Some comments on the criteria.

A) Most developers that use Calendar APIs are not trained on the subtleties of different calendars. The API to support the national calendars should be more defensive and take greater measures to help developers avoid mistakes. Since the case for different numbers of months is consistently raised as a potential pitfall, it would make sense for the API to have typed month instances and reduce the reliance on numeric months. The month instance could further prevent misuse by removing any notion of next or previous. If any computation is needed for months it should be done on the full date. Also remove any idea that months know how many days are in the month, etc. The ISOMonth could retain they features if that was useful.

B) The same arguments as above, the API should provide more help not less, make the functions easy to use that are unlikely be misused. But do not make the entire API hard to use.

C) The internal state is not an issue of the API design and the semantics of the types must not reflect the representation. The Chronology of LocalDate is implicit, adding access to an explicit Chronology instance for ISO does not change the semantics nor the state, the LocalDate does not need to store a reference to the Chronology because it is fixed.

D) No argument about the semantics of equals and compareTo, they have semantically important definitions. The toString method is defined for convenience in debugging but its value does not need to follow rigid format rules. If it is more useful to developers without the Chronology then that's ok. The toString value is used by developers as a convenient shortcut for the value but it too needs a useful and correct value but for complete control the formatter is expected to be used.

E) A certain economy of classes and packages is desirable but as long as each package and class has a clear and appropriate use, the number of classes is not the most important design factor.

F) This criteria completely backward, every API should be the best API for the purpose. As described above for A) the calendar neutral API should be the best designed possible for its developers and use cases. [For The Java runtime and Oracle products with internationalization requirements this is essential.]

Missing Criteria,

G) APIs should be orthogonal and not duplicate or have minimally different semantics. Common semantics should be represented in the type system so it is clear what is a general contract and what is specific behavior.

H) Type safety and the value static typechecking that can help reduce coding errors.

Include also the A+ requirement in the previous comment.

RogerRiggs commented 12 years ago

Comments on options:

1) ChronoDate

2) LocalDate extending ChronoDate.

3) Functionality only on Chronology

4) No classes - Parameterized functions

5) Chrono wrapper

jodastephen commented 12 years ago

For reference, here is the CLDR data on calendar preferences:

<calendarPreferenceData>
  <calendarPreference territories="001" ordering="gregorian"/>
  <calendarPreference territories="AE BH DJ DZ EH ER IQ JO KM KW LB LY MA MR OM PS QA SA SD SY TD TN YE" ordering="gregorian islamic islamic-civil"/>
  <calendarPreference territories="AF IR" ordering="gregorian persian islamic islamic-civil"/>
  <calendarPreference territories="CN CX HK MO SG" ordering="gregorian chinese"/>
  <calendarPreference territories="EG" ordering="gregorian coptic islamic islamic-civil"/>
  <calendarPreference territories="ET" ordering="gregorian ethiopic"/>
  <calendarPreference territories="IL" ordering="gregorian hebrew islamic islamic-civil"/>
  <calendarPreference territories="IN" ordering="gregorian indian"/>
  <calendarPreference territories="JP" ordering="gregorian japanese"/>
  <calendarPreference territories="TH" ordering="buddhist gregorian"/>
  <calendarPreference territories="TW" ordering="gregorian roc chinese"/>
</calendarPreferenceData>

As can be seen, the only country where the gregorian is not viewed by CLDR as the preference is Thailand. Web searches suggest that many businesses in Thailand use the ISO calendar.

jodastephen commented 12 years ago

Examples of uses of java.util.Calendar:

Example of broken logic, assuming calendar is ISO:

Example of inherited logic that is done reasonably:

Successfully only using GregorianCalendar:

Assigning variable unecessarily to Calendar - future maintainer may change from new GregCal to Cal.getInstance and break code:

Just really bad code:

No calendar date logic, just access getTime() or similar:

Evidence that inherited calendar system code is a Really Bad Idea.

dchiba commented 12 years ago

Please do not separate the API. It goes against the common OO practice to define classes dedicated to ISO. It would be easier to understand for developers to have all calendars as a subclass of a generic interface, as in java.util.GregorianCalendar that extends java.util.Calendar.

For software internationalization, conventionally, the application executes a single code path that uses a uniform interface. It is unexpected and quite undesirable for an application to have to use a special code path to support a non-ISO calendar. That would create confusion as to which API to use and how. I do not understand why we should give up the conventional OO principle. Please include in the criteria to provide the functionalities consistently for all calendars through a uniform interface.

I think the .NET framework introduced complexity by defining the redundant DateTime object. It forces developers to deal with more interfaces than necessary. ISO is certainly very special, but it does not require a separate API. Java should keep API simple.

The CLDR definition for the preferred calendars per territory indicates that in several parts of the world some non-Gregorian calendars as well as the Gregorian calendar are used. Therefore, what is useful for ISO is useful for non-ISO and separation does not make good sense. Please unify the API.

jodastephen commented 12 years ago

A note to readers of this thread that dchiba is Dan Chiba, member of Oracle's I18N team.

Firstly, 310 does not require two code paths, one for ISO and one for non-ISO. The Chrono classes are for multi-calendar, and the standard classes for the 90%+ of applications that do not need alternate calendar systems.

The .NET framework and 310 make a distinction between the concept of a date and the representation of a date. In both cases, the concept class (DateTime in .NET or LocalDate in 310) have additional methods to support the common ISO access. In neither case does this compromise the representation.

As I have indicated elsewhere, one key problem here is that most users have no experience in I18N calendars and get it wrong by making invalid assumptions (such as number of months in a year, or days in a month). It is also vital to understand how the existing Calendar class is misused in the links above.

Developers in Java have an ingrained desire to use high level classes/interfaces as variable types, as per List list = new ArrayList. For calendars, this results in code that is like Calendar cal = new GregorianCalendar, or what would be ChronoDate date = LocalDate.now. In the calendar cases, what then happens is that developers write code that makes ISO assumptions, because most developers have no experience of other calendars. Developers also tend to write methods taking in that high level type - calculateSomething(Calendar) which they use with ISO assumptions. In both cases, the applications suffer from "false I18N". The developer believes they have done a good job by using Calendar and not "hard coding" GregorianCalendar, yet their application is still just as ISO limited as it would have been. The basic model of GregorianCalendar extending Calendar is a core part of the problem with the current JDK.

In summary, date and time is a hard concept that most people, including developers, think is easy. The 310 API works really hard to ensure that application bugs are designed out for the common case. Inheriting classes adds complexity and introduces many bugs to the common case which the vast majority of users care about.

jodastephen commented 12 years ago

A comment about earlier suggestions (RogerRiggs) for extending type safety in non-ISO calendars. It is my understanding that the majority of use of non-ISO calendars is in I18N applications. As such, those applications know nothing about any specific type associated with the calendar being used, and solely access through the generic API (Calendar today). Given this, additional types that are specific to a calendar system don't add much value (which is why I consider the current Era interface and implementations to be of dubious value).

Were there to be a generic Month class, that would similarly add little additional value. The range method will specify the boundaries of valid values, and the general get, with, plus methods allow query and manipulation in a safe manner. In combination they provide all that is necessary.

RogerRiggs commented 12 years ago

A generic month interface will be needed to provide a consistent API to developers for the necessary functions mentioned above plus the internationalization/localization of names and formatting information. For months and eras, there need to be mechanisms to convert between the indexes and the instances. If compile time constants are needed, they can be declared or found by lookup by name. They do not need to be enumerations, the only additional value of enumerations is the ability to switch; which is of limited value for Eras, though perhaps a bit more useful for months.

dchiba commented 12 years ago

Sorry it was rude of me to jump into the discussion without introducing myself to everyone here. I posted an introduction to the threeten-develop list.

http://sourceforge.net/mailarchive/message.php?msg_id=29796555

With separate APIs, an application that needs to support non-ISO calendars is forced to use a fairly different design from the design of a functionally equivalent ISO only application. This is a major problem because then it would be very difficult to build a properly internationalized application. Enabling non-ISO calendar support should require minimal code changes. This is akin to the char datatype being Unicode enabled and generic for supporting all languages of the world so it usually does not require major changes for an originally single byte application to become multibyte enabled.

If the API is unified and the functionalities are provided through the same interface for all calendars, the ease of building a non-ISO enabled application would dramatically improve. In the real world ISO is one of the calendars so why not in Java. It would be easy for Java developers to understand the minor differences between calendars if they are modeled using type hierarchy.

It is not a problem for an application developer to make ISO assumptions and as a result the application only works for the ISO calendar, unless it must support a non-ISO calendar. As a matter of fact, an application that supports non-ISO as well as ISO calendars often needs a part of the code where ISO calendar is used exclusively. In many cases certain operations such as exchanging datetime values with another component and storing them should be performed in ISO only. It is true that ISO assumptions can be a problem, but I don't think they can be a good reason to separate the API.

RogerRiggs commented 11 years ago

To make some of the issues more concrete, I prototyped two aspects 1) the completeness of the Chrono API and 2) Option 2 of Issue #48 about the relationship of ISO to Chrono.

1) The Chrono API is missing common necessary functions found in the ISO API including support for combinations of dates and times. Types are added for ChronoDateTime, ChronoOffsetDateTime, ChronoZonedDateTime. The Month type is changed to an interface and simplified to be useful in the more general calendar case. The ISO enumeration of months does not need to be public but could be. The pre-defined Months (July) are defined in LocalDate; they are convenient to access as constants but logically scoped to the ISO calendar.

Option 2 proposes to make the Local (ISO) types subtypes of the more general Chrono types sharing semantics and implementation where appropriate. The ISO types are modified to inherit from Chrono types.

See: http://cr.openjdk.java.net/~rriggs/threeten-option2/ Javadoc diffs are in http://cr.openjdk.java.net/~rriggs/threeten-option2/diffs

RogerRiggs commented 11 years ago

With the discussion and inclusion of the Multi-Calendar API changes I think this can remove the label disagreement, design and marked it approved and closed.