Feature #9549: Improvements to Time::strptime

• Status: Rejected
• Assignee: akr (Akira Tanaka)
• Target version: 2.2.0

[sferik (Erik Michaels-Ober)]

I opened a pull request on GitHub a few days ago but wanted to open a parallel issue here, since I think this is the preferred tracker.

This patch includes three commits:

• Add default arguments to Time::strptime to match Date::strptime. After this patch, Time::strptime may be invoked with 0, 1, or 2 arguments. When invoked with 0 or 1, it produces a result equivalent to Date::strptime invoked with the same arguments.

• Raise ArgumentError if time passed to Time::strptime is invalid. This fixes a Ruby bug and adds a test to ensure it will not regress. Before this commit:

  require 'date'
  Date::strptime('31/02/2014', '%d/%m/%Y') # ArgumentError: invalid date
  require 'time'
  Time::strptime('31/02/2014', '%d/%m/%Y') # 2014-03-03 00:00:00 +0000

• I have also renamed variables in Time::parse to be consistent with the changes I made to Time::strptime. Specifically, renaming d to hash, since it is not a Date and renaming the date parameter to time. I believe both of these changes make the code clearer.

Thank you for considering this patch.

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Open to Assigned
• Assignee set to akr (Akira Tanaka)

[akr (Akira Tanaka)]

• Status changed from Assigned to Feedback

Erik Michaels-Ober wrote:

• Add default arguments to Time::strptime to match Date::strptime. After this patch, Time::strptime may be invoked with 0, 1, or 2 arguments. When invoked with 0 or 1, it produces a result equivalent to Date::strptime invoked with the same arguments.

I'm not sure that is a good idea.
For example, -4712-01-01 is not a special date in Time.

• Raise ArgumentError if time passed to Time::strptime is invalid. This fixes a Ruby bug and adds a test to ensure it will not regress. Before this commit:

  require 'date'
  Date::strptime('31/02/2014', '%d/%m/%Y') # ArgumentError: invalid date
  require 'time'
  Time::strptime('31/02/2014', '%d/%m/%Y') # 2014-03-03 00:00:00 +0000

I'm reluctant to such a restriction.

I tried to verify invalid time using round trip test once.
(ruby-core:14517, ruby-dev:33058, r14765, r15203)

But it caused bigger problems over benefits.

[akr (Akira Tanaka)]

• Related to Bug #10588: Invalid Dates added

[akr (Akira Tanaka)]

• Status changed from Feedback to Rejected