Time type alias defined in different places

[jvoigtlaender]

There is one definition of

type alias Time = Float

in Time.elm and an identical one in Task.elm.

This seems conceptually wrong, and potentially hurtful for later developments. So I think it would be better to have that definition only in one place, and import it everywhere else it is needed.

What would be options for this?

• One natural place to put the single definition of Time would be Time.elm, but that is problematic, since Time.elm imports Signal.elm, which imports Task.elm, so having Task.elm import Time.elm would lead to a cycle.
• The definition could be put into Task.elm and Time.elm could import it from there. It seems a bit odd, though, to have the primary definition of Time (and the place where it is documented) be in Task.elm.
• The definition could be put into Basics.elm and be imported into Time.elm and Task.elm from there.

Is anything wrong with the third option? It would essentially give Time the same status as Int, Float, String as pre-defined Elm types. It seems that is an okay thing to happen.

[mgold]

Task does not expose the Time alias, and the compiler can figure out what type it wants, so I don't see much of a downside to the current arrangement. I suppose I could get behind placing it in Basics, if that wouldn't make Task and Time much more complicated, but again I don't see the need to.

[mgold]

Task does not expose the Time alias, and the compiler can figure out what type it wants, so I don't see much of a downside to the current arrangement. I suppose I could get behind placing it in Basics, if that wouldn't make Task and Time much more complicated, but again I don't see the need to.

[jvoigtlaender]

It's not a big issue, of course. More a matter of principle and style and looking into possible futures. Consider:

• At some point it is decided that Time should not be Float, but maybe Int or even some completely other type. Then there will be two places where this has to be changed. No big problem, I'll grant, because at least these two places are both in the same package core, so the compiler would likely catch this before release. (Actually, it would only catch it if there is some code in core in which Task.Time and Time.Time are unified. It might be the case that there is no such code. Then, core would compile just fine and could be released, but clients couldn't use the modules as intended, because they wouldn't be able to pass Times created with functions from the Time module to functions from the Task module that expect a Time.)
• Or (and more likely, since it has always been said that this is coming), at some point something like Haskell's newtype feature is implemented via union types with exactly one constructor in Elm. Then certainly we will want to see type Time = Time Float (we should want this already now, for the same type safety reasons that usually motivate newtype in Haskell libraries). And then your argument vanishes. We will then have to put the Time definition in exactly one place. Which, at that time in the future, will force refactoring of more code than it does today.

So I stand with: This should be done. Better now than later.

Relatedly, this was done.

[jvoigtlaender]

It's not a big issue, of course. More a matter of principle and style and looking into possible futures. Consider:

• At some point it is decided that Time should not be Float, but maybe Int or even some completely other type. Then there will be two places where this has to be changed. No big problem, I'll grant, because at least these two places are both in the same package core, so the compiler would likely catch this before release. (Actually, it would only catch it if there is some code in core in which Task.Time and Time.Time are unified. It might be the case that there is no such code. Then, core would compile just fine and could be released, but clients couldn't use the modules as intended, because they wouldn't be able to pass Times created with functions from the Time module to functions from the Task module that expect a Time.)
• Or (and more likely, since it has always been said that this is coming), at some point something like Haskell's newtype feature is implemented via union types with exactly one constructor in Elm. Then certainly we will want to see type Time = Time Float (we should want this already now, for the same type safety reasons that usually motivate newtype in Haskell libraries). And then your argument vanishes. We will then have to put the Time definition in exactly one place. Which, at that time in the future, will force refactoring of more code than it does today.

So I stand with: This should be done. Better now than later.

Relatedly, this was done.

[evancz]

I think your point number 2 is wrong. I want to be able to say (4 * hour) so wrapping things up with type Time = Time Float seems bad given how certain things work now. It'd at least be a major undertaking to get this working nicely again.

I think point 1 is pretty weak. I think we can remember this kind of thing. If we don't remember it, we can fix it really easily.

[evancz]

I think your point number 2 is wrong. I want to be able to say (4 * hour) so wrapping things up with type Time = Time Float seems bad given how certain things work now. It'd at least be a major undertaking to get this working nicely again.

I think point 1 is pretty weak. I think we can remember this kind of thing. If we don't remember it, we can fix it really easily.

[jvoigtlaender]

Now that's a valid point, against type Time = Time Float. I had forgotten that the stuff like hour in Time are just constants, not functions like hours : number -> Time. So yes, if we want to write stuff like (4 * hour), and don't have type classes, wrapping Time as a newtype style thing is not an option.

And yes, as I already granted, the other point about potentially conflicting changes is not a major thing. It's at least likely it won't hurt much.

Anyway, I still think it would make sense to put Time into Basics. Isn't Time meant to be the "standard" type for this in Elm? We don't have to import module String to get Elm's String type into scope (same for lists). So why do we have to import module Time to get Elm's Time type into scope? Ultimately, the current setup tempts people to just use Float instead of Time in their own type signatures even where Time would be appropriate and even if they eventually want to pass those values on to something like Task.sleep. (They are tempted to do that because thus they can save the import Time exposing (Time) line.) Having Time defined in Basics, and thus always in scope, would remove that temptation, which would result in better user code.

The fact that we wouldn't have two definitions of Time in core would be a nice side effect.

But feel free to close this issue if you really prefer the current setup.

[jvoigtlaender]

Now that's a valid point, against type Time = Time Float. I had forgotten that the stuff like hour in Time are just constants, not functions like hours : number -> Time. So yes, if we want to write stuff like (4 * hour), and don't have type classes, wrapping Time as a newtype style thing is not an option.

And yes, as I already granted, the other point about potentially conflicting changes is not a major thing. It's at least likely it won't hurt much.

Anyway, I still think it would make sense to put Time into Basics. Isn't Time meant to be the "standard" type for this in Elm? We don't have to import module String to get Elm's String type into scope (same for lists). So why do we have to import module Time to get Elm's Time type into scope? Ultimately, the current setup tempts people to just use Float instead of Time in their own type signatures even where Time would be appropriate and even if they eventually want to pass those values on to something like Task.sleep. (They are tempted to do that because thus they can save the import Time exposing (Time) line.) Having Time defined in Basics, and thus always in scope, would remove that temptation, which would result in better user code.

The fact that we wouldn't have two definitions of Time in core would be a nice side effect.

But feel free to close this issue if you really prefer the current setup.

[evancz]

I think having two is dumb as well, so to get back on track: is the proposal to put it in Basics now? The other options are filtered out?

[evancz]

I think having two is dumb as well, so to get back on track: is the proposal to put it in Basics now? The other options are filtered out?

[jvoigtlaender]

Yes, I think so.

[jvoigtlaender]

Yes, I think so.

[evancz]

Let's either open an issue that describes the refined idea clearly or do a PR.

[evancz]

Let's either open an issue that describes the refined idea clearly or do a PR.

[jvoigtlaender]

I've prepared such a PR: https://github.com/elm-lang/core/pull/415. (I've checked it locally with an example program that uses both Time.second and Task.sleep as well as an explicit type signature including Time. All I needed to do to make the program work with the revised core as per that pull request was to remove Time from the list in my import Time exposing (Time, ...) line. If I hadn't been using Time.second in that program, I could now have removed the whole import Time ... line.)

[jvoigtlaender]

I've prepared such a PR: https://github.com/elm-lang/core/pull/415. (I've checked it locally with an example program that uses both Time.second and Task.sleep as well as an explicit type signature including Time. All I needed to do to make the program work with the revised core as per that pull request was to remove Time from the list in my import Time exposing (Time, ...) line. If I hadn't been using Time.second in that program, I could now have removed the whole import Time ... line.)