eclipse-archived / ceylon-sdk

Standard platform modules belonging to the Ceylon SDK
http://www.ceylon-lang.org
Apache License 2.0
72 stars 60 forks source link

Update all numerical time representations to use Instant and Duration instead #137

Open ncorai opened 10 years ago

ncorai commented 10 years ago

Some occurrences: File.lastModifiedMilliseconds Session.timeout Session.creationTime Session.lastAccessedTime WebSocketChannel.idleTimeout TestResult.elapsedTime TestRunResult.startTime TestRunResult.endTime TestRunResult.elapsedTime

quintesse commented 10 years ago

Now what I was thinking is that maybe we could make Instant and Duration part of the language module. That way instead of having these unknown Integer quantities we would have something we know represents either a point in time or a time period.

And if we then choose to use a nano-second representation we would have the best possible "spread" of useful applications.

The benefit is that you'd never have to look up what WebSocketChannel.idleTimeout uses as a basis for its timing (is it seconds? millseconds? minutes?).

We get back the usefulness of an actual class with clear meaning but without dragging in the entire time module.

To me this seems pretty cool. Wdyt: @DiegoCoronel @luolong @gavinking ?

quintesse commented 10 years ago

PS: system.milliseconds would become an Instant, but system.nanoseconds would remain an Integer because it's not a clock. But you could use it for duration calculations like value d = Duration(system.nanoseconds - oldnanoseconds) if you wanted.

quintesse commented 10 years ago

PS2: looking at the code it would mean moving the methods that turn an Instant into a Time or Date out of it. But I don't think that's too much of a problem. They could be made top levels or so.

ncorai commented 10 years ago

To me, time-keeping is an essential part of any development I've ever done, so I do agree that machine time belongs in the language module and Ceylon.time could be used for human representation.

I've worked with JSR 310 for years and had to deal with most other APIs not making use of it, of course. Having to manually wrap/unwrap wherever Instant is not usable can get old quickly.

DiegoCoronel commented 10 years ago

Well, i dont know if it would be really useful, from my point of view make Instant part of language means remove some methods like date, dateTime, zoneDateTime and change minus/plus removing period from it. Then you will know that File.lastModifiedMilliseconds is an Instant but you can´t manipulate it like a date naturally, and now you will need to know all top level methods to get a DateTime/Date/Time/ZoneDateTime from an Instant that only remains with millisecondsOfEpoch.

//Currently we can do
 Instant lastModified = Instant( File.lastModifiedMilliseconds );
 lastModified.dateTime();
 lastModified.date();
 lastModified.time();
 lastModified.plus(someDuration).dateTime();
.
.
//Changing it we are going to do
 Instant lastModified = File.lastModified;
 someTopLevelConverterToDateTime( lastModified );
 someTopLevelConverterToDate( lastModified );
 someTopLevelConverterToTime( lastModified );
 someTopLevelConverterToDateTime( lastModified.plus(someDuration) );

I think that this way Instant will be much less expressive. Maybe we need some class to express just this time in ceylon.language, i dont know if the best name is Instant or if we can find something better and of course we could support it at ceylon.time

gavinking commented 10 years ago

Yes, my reaction was similar to Diego's:

It's likely that either:

or else we end up with just this braindead Instant that is no better than an Integer.

drochetti commented 10 years ago

Somehow, this makes me think about extension methods again...

quintesse commented 10 years ago

@DiegoCoronel sure, I understand that, we'd have to see if there is a way to not loose the expressiveness and ease-of-use of ceylon.time of course. But think about what you'd gain: a much more direct coupling with a) the rest of the SDK and b) a standard way for people to define times and durations in their APIs!

Because even we don't use it in our own SDK (!) and likewise others won't want to pull in a dependency on ceylon.time either just because they have a single timeout or dateModified value somewhere.

@gavinking But even a brain-dead Instant would be much better than a simple Integer! Like I mentioned, you can't know what the Integer is without looking at the documentation. That's something that we've always tried to avoid.

I think @ncorai has a very good point when he singles out those methods that take Integer in the above list. I have no idea what to expect. I assume that all the ones that represent a time are using Java's default but the ones that are periods? I have no clue until I read the docs.

To me it seems like an opportunity to make something really useful. Yes, it might not be immediately obvious how to do it, it's not as simple as just moving two classes to ceylon.language (which is a bit of the max number that would be allowed), but that doesn't seem like a good reason not to do it given what we could gain by it.

gavinking commented 10 years ago
shared alias MillisecondsInEpoch => Integer; 
ncorai commented 10 years ago

I see that Ceylon.net has dependencies on Ceylon.io, Ceylon.process on Ceylon.io and Ceylon.file. I understand that you guys want to minimize interdependencies but, since there are no external dependencies in Ceylon.time itself, it should be fine using it in other modules, no?

ncorai commented 10 years ago

Anyway, feel free to discard that bug. I thought it would make sense to encapsulate integers into more meaningful time but I understand my proposal has undesirable side effects.

quintesse commented 10 years ago

@gavinking That's almost like using a scripting language ;)

gavinking commented 10 years ago

@ncorai we haven't rejected the idea yet. I especially want to hear what @luolong thinks.

luolong commented 10 years ago

I am actually going to side with @gavinking and @DiegoCoronel with this one. While on the surface it sounds good, it would either mean pulling in the entire module into c.l or having the confusion of two Instants in lang and sdk.

I do understand that the issue is that sdk modules should use these types, so why can't they simply import the ceylon.time module and use these types directly?

quintesse commented 10 years ago

@luolong So you're actually not siding with them, at least not completely, because you think values that represent time should use the rich interface provided by ceylon.time.

So that either means you convince people that having dependencies on ceylon.time is not a problem or you'll have to accept that we'll use Integer all over the place (which to me seems horrid).

luolong commented 10 years ago

Yes, I believe that using richer and more semantic types for time and duration in SDK is much preferable to using Integers all over the place.

I firmly believe that Instant is stable enough to be used everywhere it makes sense.

Now, who do I need to convince and how should I go about doing it?

ncorai commented 10 years ago

If I'm allowed to make a parallel with Java, pre-8 pretty much every class that dealt with time in the JDK was relying on java.util.Date, from Swing to JDBC.

Also, I'm personally not fond of having two versions of Instant (language and time), dealing with auto imports of java.sql.Date instead of java.util.Date was enough bother the first time around.

@luolong: I was thinking of the dependencies on DateTime and such that Instant brings along. How disruptive for the ceylon.time package would it be to for instance replace Instant.dateTime(TimeZone=) with a top-level function such as dateTimeFrom(Instant, TimeZone=) in the DateTime.ceylon file? I'm asking considering DateTime already has a dependency on Instant due to the DateTime.instant(TimeZone=) method, so it would actually remove a cycle.

ncorai commented 10 years ago

@DiegoCoronel: per your earlier objection to what I suggested above, even after removing date(), time() and dateTime(), if you keep all the operations and the interactions with Duration, Instant would still be pretty useful, far more than a dumb Integer value would be.

luolong commented 10 years ago

@ncorai: It is certainly technically possible, but not something I would personally like. We've been fighting hard to cut the number of top-level functions to minimum in ceylon.time -- it does keep the overall API much cleaner, I think.

It would be quite disruptive to the API design. Technically, it is not a big deal, but the API would suffer.

quintesse commented 10 years ago

By coincidence I saw this part of a Scala talk that exactly represents how I feel about this: http://youtu.be/TS1lpKBMkgg?t=8m30s (2 minutes long).

"We are using a language with types and we've essentially abandoned it".

I agree strongly with him on this and feel that we cannot do something similar. So in my opinion Integers used as time values must go. The way we do that is still open for discussion, though.

thradec commented 10 years ago

here's my 2 cents, from user point of view, it's pretty lame, that this ultra type-safe language expose time like number in whole sdk(except one module), so we should try to find nicer way

luolong commented 10 years ago

So, what is the deal with depending on ceylon.time in the SDK modules?

Or are there some classes in the language module that need this as well?

quintesse commented 10 years ago

Well system.milliseconds of course :)

But I think that in the language module that's about it.

I think that for the rest of the SDK it's just the idea to keep things as loosely coupled as possible. Not much use to make a modularized SDK when (using red herring argument) importing one module imports them all.

gavinking commented 10 years ago

So, what is the deal with depending on ceylon.time in the SDK modules?

That's not really the issue.

Or are there some classes in the language module that need this as well?

That is the issue.

quintesse commented 10 years ago

Btw, it would also mean that we would have a common basis if in the future somebody wants to make a different framework. Instant and Duration are so universally usable that somebody could make an entirely different time/calendar framework and people could use it without any problem, exaclty because File.lastModified would not (should not?) be tied in any way to ceylon.time.

DiegoCoronel commented 10 years ago

@quintesse @ncorai im not against have a more safe representation of time in ceylon.language, im only worried if just Instant is enough for it and also about been less expressive. And later developers will ask why do they dont have something like Seconds, Minutes, Hours ... to represent for example Session.timeout or maybe they ask for a better formatter to Instant in ceylon.language. So, for me the problem is find the right limit for it.

I think it would fit better if ceylon add extension methods as @drochetti said ;)

luolong commented 10 years ago

Well, if put this way, it makes a whole lot of sense to pull the Instant, Duration (and Clock, I must add) out of ceylon.time and into the language module.

I guess, we can live with the little inconvenience of top-level functions operating on ceylon.language:Instant to get to Date, Time and friends.

Just playing along with the idea of the new top-level functions in this reply (given that Instant is in ceylon.language) to get the feel of how it could potentially look like:

Instant instant = ...
Date date = dateOf(instant, timeZone);
Time time = timeOf(instant, timeZone);
Instant sameInstant = date.instant(time, timeZone);
assert(instant == sameInstant);

Actually, writing this, I had another question occur to me. Since the concept of TimeZone is closely tied to a locale, would that also mean that TimeZone would have to move to the language module?

quintesse commented 10 years ago

@luolong

Since the concept of TimeZone is closely tied to a locale, would that also mean that TimeZone would have to move to the language module?

Let's leave that discussion for when we start on ceylon.locale ;)

The Clock doesn't seem to be as necessary to have in c.l , right?

@DiegoCoronel

And later developers will ask why do they dont have something like Seconds, Minutes, Hours ...

Indeed, in fact the Duration class is only useful with at least the most common durations available. I'm not sure where they are no (I didn't find any in a quick search I did), but I was thinking along the line of a toplevel object:

shared object durations {
    Duration nanosecond = Duration(1);
    Duration microsecond = Duration(1000);
    Duration millisecond = Duration(1000000);
    Duration second = Duration(1000000000);
    Duration minute= Duration(60000000000);
    Duration hour = Duration(3600000000000);
    Duration day = Duration(86400000000000);
}

I think that would give us enough for most cases?

quintesse commented 10 years ago

Or just make one toplevel object called time, put those durations there as well as move milliseconds and nanoseconds from system over to it. That way we limit everything in c.l to 2 classes and 1 object. I don't think we can do any better than that.

DiegoCoronel commented 10 years ago

huum, isnt JS that only uses 2^53 to represent numbers? In this case making Duration with a nanoseconds precision it can make it useless for JS, is it? or my calcs are wrong? (im really with doubt here)

quintesse commented 10 years ago

It would give us 285 years which would seem enough.

Do you guys support arbitrary dates? Like 3000 years ago for example? And if so, do you somehow use Instant for that?

DiegoCoronel commented 10 years ago

For now we have limited it to:

shared object years satisfies UnitOfYear {
    shared Integer minimum = -283_456; 
    shared Integer maximum = 287_396;
}

Instant isnt necessary to get the date, just one way for that.

P.S: We need to deal with the right chronology yet, currently we are converting everything to Gregorian Calendar.

gavinking commented 10 years ago

Aren't we now arriving at exactly what I predicted? We started with Instant, then Duration, now we're wondering about Locale and TimeZone. Hey, who needs modularity when we can just stick everything in ceylon.language. Sure, that's a little snarky, but there's a serious point behind it.

quintesse commented 10 years ago

Well it does not have to do with this issue directly so I'd rather not muddy the waters.

But if we see that we're using Strings to pass Locales around because we don't want to depend on ceylon.locale then I'd definitely say that we've hit the same snag.

ncorai commented 10 years ago

Since Roland was considering adding the Date dateOf(Instant, TimeZone=) top-level function to the ceylon.time package, where Date date(Integer, Integer|Month, Integer) already resides, it means the dependency on locale is actually in the ceylon.time module. If you move Instant and Duration out of that module, either in ceylon.language or in a dedicated ceylon.machinetime module, then you avoid dependencies on ceylon.locale.

ncorai commented 10 years ago

Oh, I get it now, you were referring to process.locale and process.timezoneOffset, not to any Instant methods.

In my view, every concept exposed by ceylon.language should be well defined, not left as a number or string to parse. With Ceylon, an instance of a type is by definition valid (since it can't be null, which is awesome), unless it's actually a String that needs to be parsed. In a statically-typed language, strings should really be restricted to parsing input and formatting output, because they are in essence duck-typing. I greatly increased the reliability of my projects over the past few years by replacing numbers and strings in my APIs with actual types.

So I'd go the extra-mile and say that, if you don't want to include Locale in ceylon.language, then remove process.locale and put that information in a top-level object of the ceylon.locale module, because really code that will request that info has to be aware of localization, so it may as well acknowledge it.

And yes, I am aware that it's easy to speak theoretically and that I'm not the one dealing with the constraints of reality. Even if you discard this bug, I'll still use Ceylon of course.

quintesse commented 10 years ago

@ncorai I'm not really worried about process.locale and process.timezoneOffset right now to be honest, we need them now for technical reasons because we can't put them inside ceylon.time or ceylon.locale yet until we have conditional compilation for specific backends figured out. Once we do, we can remove them form the language module and move them to where they belong.

luolong commented 8 years ago

I am reading this thread through and now that we have native annotation support, we can actually get rid of things like process.timezoneOffset and friends. Separate question is should we?