ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: "timcraft (Tim Craft)" <noreply@ruby-lang.org>
To: ruby-core@ruby-lang.org
Subject: [ruby-core:106036] [Ruby master Feature#18033] Time.new to parse a string
Date: Fri, 12 Nov 2021 11:33:24 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-94627.20211112113323.4@ruby-lang.org> (raw)
In-Reply-To: <redmine.issue-18033.20210709061300.4@ruby-lang.org>

Issue #18033 has been updated by timcraft (Tim Craft).


Nice performance improvement!

In terms of the interface I think it would be confusing to make `Time.new` parse a string:
* Using a `.parse` method for parsing is more conventional / less surprising / more intention revealing
* Having *both* `.new` and `.parse` would be confusing because it's not clear when to use one or the other, what the trade-offs are etc

It feels very unintiutive and not "least surprise". Similarly `Time.parse(...,format: :iso8601)` and `Time.parse_iso8601` feel a bit clunky.

Is this awkwardness stemming from trying to fit much functionality into the Time class? It would be more work, but if there was a dedicated ISO8601 module we could shift some responsibility away from the Time class. For example:

```
ISO8601::Time.parse  # strictly ISO8601 parsing, returns a Time object
```

This could be extended to encapsulate ISO8601 parsing for other classes:

```
ISO8601::Date.parse
ISO8601::Duration.parse
ISO8601::TimeInterval.parse
```

The same general pattern could be followed for other formats:

```
RFC3339::Time.parse
RFC2822::Time.parse
SQL92::Time.parse
```

Each of those methods would have the same signature, so they could be easily interchanged to get varying degrees of strictness or performance.

From a naming and readability perspective the names are less surprising, more intention revealing, and very greppable.

The higher level `Time.parse` method could potentially delegate to these methods to support parsing a wider range of formats.

Each module could also be extended to encapsulate string formatting as an alternative to adding instance methods like #iso8601, #rfc3339 etc.


***

An alternative to using the formats as the top level modules would be to invert and nest the modules under the Time classes, for example:

```
Time::ISO8601.parse  # instead of ISO8601::Time.parse
```

Same benefits—I'm not sure which would be considered more Ruby-ish?

----------------------------------------
Feature #18033: Time.new to parse a string
https://bugs.ruby-lang.org/issues/18033#change-94627

* Author: nobu (Nobuyoshi Nakada)
* Status: Open
* Priority: Normal
----------------------------------------
Make `Time.new` parse `Time#inspect` and ISO-8601 like strings.

* `Time.iso8601` and `Time.parse` need an extension library, `date`.
* `Time.iso8601` can't parse `Time#inspect` string.
* `Time.parse` often results in unintentional/surprising results.
* `Time.new` also about 1.9 times faster than `Time.iso8601`.

    ```
    $ ./ruby -rtime -rbenchmark -e '
    n = 1000
    s = Time.now.iso8601
    Benchmark.bm(12) do |x|
      x.report("Time.iso8601") {n.times{Time.iso8601(s)}}
      x.report("Time.parse") {n.times{Time.parse(s)}}
      x.report("Time.new") {n.times{Time.new(s)}}
    end'
                       user     system      total        real
    Time.iso8601   0.006919   0.000185   0.007104 (  0.007091)
    Time.parse     0.018338   0.000207   0.018545 (  0.018590)
    Time.new       0.003671   0.000069   0.003740 (  0.003741)
    ```

https://github.com/ruby/ruby/pull/4639



-- 
https://bugs.ruby-lang.org/

  parent reply	other threads:[~2021-11-12 11:33 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09  6:13 [ruby-core:104552] " nobu
2021-07-09  6:30 ` [ruby-core:104554] " samuel
2021-07-09  7:51 ` [ruby-core:104559] " nobu
2021-07-09  8:29 ` [ruby-core:104562] " samuel
2021-07-09  8:51 ` [ruby-core:104564] " nobu
2021-07-10  8:42 ` [ruby-core:104575] " jean.boussier
2021-07-16  2:48 ` [ruby-core:104626] " samuel
2021-10-24 23:14 ` [ruby-core:105763] " ioquatix (Samuel Williams)
2021-11-10 10:44 ` [ruby-core:106002] " Eregon (Benoit Daloze)
2021-11-10 11:26 ` [ruby-core:106007] " byroot (Jean Boussier)
2021-11-12 11:33 ` timcraft (Tim Craft) [this message]
2021-12-03  4:07 ` [ruby-core:106440] " nobu (Nobuyoshi Nakada)
2021-12-03  6:54 ` [ruby-core:106443] " nobu (Nobuyoshi Nakada)
2021-12-03 10:08 ` [ruby-core:106449] " sawa (Tsuyoshi Sawada)
2021-12-03 10:23 ` [ruby-core:106451] " sawa (Tsuyoshi Sawada)
2021-12-03 11:33 ` [ruby-core:106452] " Eregon (Benoit Daloze)
2021-12-03 13:21 ` [ruby-core:106456] " nobu (Nobuyoshi Nakada)
2021-12-03 13:49 ` [ruby-core:106458] " nobu (Nobuyoshi Nakada)
2021-12-06  4:55 ` [ruby-core:106501] " nobu (Nobuyoshi Nakada)
2021-12-06 13:18 ` [ruby-core:106512] " Eregon (Benoit Daloze)
2021-12-06 14:54 ` [ruby-core:106516] " nobu (Nobuyoshi Nakada)
2021-12-06 17:37 ` [ruby-core:106521] " Eregon (Benoit Daloze)
2021-12-07 14:15 ` [ruby-core:106537] " nobu (Nobuyoshi Nakada)
2022-12-15  6:11 ` [ruby-core:111293] " mame (Yusuke Endoh)
2022-12-15  6:54 ` [ruby-core:111295] " matz (Yukihiro Matsumoto)
2024-10-11 12:49 ` [ruby-core:119511] " Eregon (Benoit Daloze) via ruby-core

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-94627.20211112113323.4@ruby-lang.org \
    --to=noreply@ruby-lang.org \
    --cc=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).