New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement `toString` for `Date` #442

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@rgrempel
Contributor

rgrempel commented Nov 21, 2015

Consider

toString (Date.fromTime 1422777600000)

Before this pull request, it results in:

{}

After this pull request, it results in:

Sun Feb 01 2015 02:00:00 GMT-0600 (CST)

@rgrempel rgrempel changed the title from Implement `toString` for `Date`. See #441. to Implement `toString` for `Date` Nov 21, 2015

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel Nov 21, 2015

Contributor

One additional consideration has now occurred to me, concerning the purity of toString in the context of a Date.

The code I submitted calls Javascript's toString() on the date. This seemed like a reasonable default string representation (as opposed to some more specific formatting of the date).

However, toString() does return a different string depending on what the Javascript runtime thinks your local timezone is. So, different computers in different timezones will get different results. Furthermore, it's probably the case that changing the timezone on your local machine would change the results. (By this, I mean manually changing what timezone you are in, via system preferences or the like).

So, I could imagine an argument that it would be better to use toUTCString() instead of toString(), since that will definitely always return the same string.

I would be happy to modify the PR if toUTCString() is considered to be better. The only reason that I don't do it immediately is that:

  • People will probably expect to see the stringified Date in the local time zone.
  • The function is only impure in odd circumstances.

However, I would certainly understand if it seemed better to avoid even an attenuated impurity.

Contributor

rgrempel commented Nov 21, 2015

One additional consideration has now occurred to me, concerning the purity of toString in the context of a Date.

The code I submitted calls Javascript's toString() on the date. This seemed like a reasonable default string representation (as opposed to some more specific formatting of the date).

However, toString() does return a different string depending on what the Javascript runtime thinks your local timezone is. So, different computers in different timezones will get different results. Furthermore, it's probably the case that changing the timezone on your local machine would change the results. (By this, I mean manually changing what timezone you are in, via system preferences or the like).

So, I could imagine an argument that it would be better to use toUTCString() instead of toString(), since that will definitely always return the same string.

I would be happy to modify the PR if toUTCString() is considered to be better. The only reason that I don't do it immediately is that:

  • People will probably expect to see the stringified Date in the local time zone.
  • The function is only impure in odd circumstances.

However, I would certainly understand if it seemed better to avoid even an attenuated impurity.

@Dremora

This comment has been minimized.

Show comment
Hide comment
@Dremora

Dremora Nov 25, 2015

It's worth noting that most functions in the Date module are impure in a way that they also depend on machine's timezone (e.g. Date.hour). For consistency, toString should also maintain similar behaviour. Alternatively, everything should be changed to ignore the time zone by switching to getUTC*() methods.

In either case many applications actually require dynamic timezone (e.g. when timezone is stored somewhere in preferences or next to the timestamp) — something that's not supported by the core library, so the choice made here, as long as it's consistent, might not be that important.

Dremora commented Nov 25, 2015

It's worth noting that most functions in the Date module are impure in a way that they also depend on machine's timezone (e.g. Date.hour). For consistency, toString should also maintain similar behaviour. Alternatively, everything should be changed to ignore the time zone by switching to getUTC*() methods.

In either case many applications actually require dynamic timezone (e.g. when timezone is stored somewhere in preferences or next to the timestamp) — something that's not supported by the core library, so the choice made here, as long as it's consistent, might not be that important.

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel Nov 25, 2015

Contributor

That's a good point -- it helps me see why my intuition was that using the local timezone for toString made sense.

Contributor

rgrempel commented Nov 25, 2015

That's a good point -- it helps me see why my intuition was that using the local timezone for toString made sense.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jan 19, 2016

Contributor

In a related issue, #481 (now closed as duplicate), @fredcy used the following test program. Might be useful over here as well:

import Date
import Effects
import Html exposing (li, text, ul)
import StartApp
import Time

app =
  StartApp.start
    { init = ( 0, Effects.none )
    , inputs = [ Time.every <| Time.second * 5 ]
    , update = \t -> always ( t, Effects.none )
    , view = always showTime
    }

showTime t =
  ul
    []
    [ li [] [ t |> toString |> text ]
    , li [] [ t |> Date.fromTime |> toString |> text ]
    ]

main = app.html 
Contributor

jvoigtlaender commented Jan 19, 2016

In a related issue, #481 (now closed as duplicate), @fredcy used the following test program. Might be useful over here as well:

import Date
import Effects
import Html exposing (li, text, ul)
import StartApp
import Time

app =
  StartApp.start
    { init = ( 0, Effects.none )
    , inputs = [ Time.every <| Time.second * 5 ]
    , update = \t -> always ( t, Effects.none )
    , view = always showTime
    }

showTime t =
  ul
    []
    [ li [] [ t |> toString |> text ]
    , li [] [ t |> Date.fromTime |> toString |> text ]
    ]

main = app.html 
@jinjor

This comment has been minimized.

Show comment
Hide comment
@jinjor

jinjor May 18, 2016

Contributor

I ran into this problem too.

The current toString does not even say it is a Date. I thought my date value was invalid. I confirmed several times that I correctly passed Unix time in millisecond and it is not before 1970. And I finally found it was a problem of toString.

Contributor

jinjor commented May 18, 2016

I ran into this problem too.

The current toString does not even say it is a Date. I thought my date value was invalid. I confirmed several times that I correctly passed Unix time in millisecond and it is not before 1970. And I finally found it was a problem of toString.

@avh4

This comment has been minimized.

Show comment
Hide comment
@avh4

avh4 May 18, 2016

Member

According to the docs, "When you view the resulting string, it should look just like the value it came from."

That suggests that toString (Date.fromTime 1422777600000) should equal "Date.fromTime 1422777600000"

But I don't think that's really what anyone here is looking for. When people want toString for Date, there are different things they usually want:

  1. view date values while debugging - they probably want to see either ISO format or the date in their locale or the date in an English locale
  2. serialize date values - they probably want ISO format, and they probably want to deserialize it later
  3. display dates in their UI - they probably want the date in the current locale, or they want more flexible formatting options

Which are we trying to address? and which of these needs is it appropriate to use toString for?

Member

avh4 commented May 18, 2016

According to the docs, "When you view the resulting string, it should look just like the value it came from."

That suggests that toString (Date.fromTime 1422777600000) should equal "Date.fromTime 1422777600000"

But I don't think that's really what anyone here is looking for. When people want toString for Date, there are different things they usually want:

  1. view date values while debugging - they probably want to see either ISO format or the date in their locale or the date in an English locale
  2. serialize date values - they probably want ISO format, and they probably want to deserialize it later
  3. display dates in their UI - they probably want the date in the current locale, or they want more flexible formatting options

Which are we trying to address? and which of these needs is it appropriate to use toString for?

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel May 18, 2016

Contributor

I don't think that we could serve purpose 3 well, since we've got no additional parameters to work with when using toString.

I would probably suggest working with Json.Encode and Json.Decode for purpose 2, since we don't, in general, have a deserializer that corresponds to toString.

Which leaves purpose 1.

But my own goal was to make toString less partial.

Contributor

rgrempel commented May 18, 2016

I don't think that we could serve purpose 3 well, since we've got no additional parameters to work with when using toString.

I would probably suggest working with Json.Encode and Json.Decode for purpose 2, since we don't, in general, have a deserializer that corresponds to toString.

Which leaves purpose 1.

But my own goal was to make toString less partial.

@fredcy

This comment has been minimized.

Show comment
Hide comment
@fredcy

fredcy May 18, 2016

Contributor

I wanted it for debugging, for easy Debug.log usage. It might serve for prototyping a UI. But any production usage for avh4's items 2 and 3 would seem to require explicit date-to-formatted-string conversion.

Contributor

fredcy commented May 18, 2016

I wanted it for debugging, for easy Debug.log usage. It might serve for prototyping a UI. But any production usage for avh4's items 2 and 3 would seem to require explicit date-to-formatted-string conversion.

@rgrempel

This comment has been minimized.

Show comment
Hide comment
@rgrempel

rgrempel Jun 24, 2016

Contributor

I have rebased this against the current master.

Contributor

rgrempel commented Jun 24, 2016

I have rebased this against the current master.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jun 26, 2016

Member

I was messing with this stuff on my flights yesterday and did something like this in 9bc52e6

Member

evancz commented Jun 26, 2016

I was messing with this stuff on my flights yesterday and did something like this in 9bc52e6

@evancz evancz closed this Jun 26, 2016

@rgrempel rgrempel deleted the rgrempel:date-to-string branch Jun 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment