Re: [Tarantool-patches] [PATCH v5 0/8] Initial datetime implementation

[Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[ += tarantool-patches - sorry dropped it from Cc by mistake ]

Putting aside minor hitches, like bad indentation here and there,
I have some concerns regarding the datetime API:

 - I believe we shouldn't introduce interval in years/months, because
   they depend on the date they are applied to. Better add datetime
   methods instead - add_years / add_months.

 - The API provides too many ways to extract the same information from a
   datetime object (m, min, minute all mean the same). IMO a good API
   should provide exactly one way to achieve a certain goal.

 - AFAICS datetime.hour returns the number of hours passed since the
   year 1970. This is confusing. I don't have any idea why anyone would
   need this. As a user I expect them to return the hour of the current
   day. Same for other similar methods, like month, minute, year.

 - Creating a datetime without a date (with parse_time) looks weird:

   tarantool> dt = datetime.parse_time('10:00:00')
   ---
   ...
   
   tarantool> tostring(dt)
   ---
   - 1970-01-01T10:00Z
   ...

   Why 1970? I think that a datetime should always be created from a
   date and optionally a time. If you want just time, you need another
   kind of object - time. After all it's *date*time.

 - datetime.days(2) + datetime.hours(12) + datetime.minutes(30)

   looks cool, but I'm not sure it's efficient to create so many objects
   and then sum them to construct an interval. It's surely acceptable
   for compiled languages without garbage collection (like C++), but for
   Lua I think it'd be more efficient to provide an API like this:

   datetime.interval{days = 2, hours = 12, minutes = 30}

   (I'm not a Lua expert so not sure about this)

 - datetime.strftime, asctime, ctime - look too low level, which is no
   surprise as they come from the C API. Judging by their names I can't
   say that they present a datetime as a string in a certain format.
   Maybe, rename strftime() to format() and make it a method of datetime
   object (datetime:format)? Without a format, it could print ASCII
   time. As for asctime, ctime, I'd drop them, because one can use
   strftime to get the same result. Besides, they append \n to the
   output, which looks weird:

   tarantool> datetime.asctime(dt)
   ---
   - 'Thu Jan  1 10:00:00 1970
   
     '
   ...