Skip to content
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

[Proposal] Temporal.Something(...args) instead of Temporal.Something.from(...args) #755

Closed
bijection opened this issue Jul 10, 2020 · 4 comments
Labels

Comments

@bijection
Copy link

bijection commented Jul 10, 2020

.from(...args) increases character count without (much) added specificity, makes the method signatures less discoverable in IDEs (since Temporal.Something will surface a suggestion for Temporal.Something()), and makes it hard to search for the documentation in search engines (e.g. a search for Temporal DateTime from could return results for Temporal DateTime, ignoring 'from').

Instead, why not allow constructors, with or without new, to take the arguments that .from takes?

For example, the set of signatures:

new Temporal.Absolute(epochNanoseconds : bigint) : Temporal.Absolute
Temporal.Absolute.from(thing: any) : Temporal.Absolute
Temporal.Absolute.fromEpochSeconds(epochSeconds: number) : Temporal.Absolute
Temporal.Absolute.fromEpochMilliseconds(epochMilliseconds: number) : Temporal.Absolute
Temporal.Absolute.fromEpochMicroseconds(epochMilliseconds : bigint) : Temporal.Absolute
Temporal.Absolute.fromEpochNanoseconds(epochNanoseconds : bigint) : Temporal.Absolute

could become

Temporal.Absolute(source: number | Date | {seconds: number} | {milliseconds: number} | {nanoseconds: number})

Similarly:

new Temporal.DateTime(isoYear: number, isoMonth: number, isoDay: number, hour: number = 0, minute: number = 0, second: number = 0, millisecond: number = 0, microsecond: number = 0, nanosecond: number = 0, calendar?: object) : Temporal.DateTime
Temporal.DateTime.from(thing: any, options?: object) : Temporal.DateTime

could become

Temporal.DateTime(source: string | Date | { 
        year: number,
        month: number,
        day: number,
        hour?: number,
        minute?: number,
        second?: number,
        millisecond?: number,
        microsecond?: number,
        nanosecond?: number,
        calendar?: object
    },
    options?: object
) : Temporal.DateTime

This pattern could be applied to most of the Temporal.* member objects, potentially increasing readability, discoverability, and brevity.

I've mentioned this in passing in #751 and #743, but thought it deserved its own issue. Let me know if there is a better forum for this suggestion, or if this has already been discussed.

@ptomato
Copy link
Collaborator

ptomato commented Jul 10, 2020

Thanks for the feedback and for proposing a change!

I believe this has been discussed at a fairly early stage in the proposal long before I was involved, but maybe someone else can weigh in, if not then I will try to dig up the discussion on Monday.

@ptomato
Copy link
Collaborator

ptomato commented Jul 13, 2020

Here are some issues that contain previous discussion on this topic:

The guiding principle so far has been, constructors are a "lower-level" way to create Temporal objects, that accept numeric-only ISO-calendar-only values, and do not take options; corresponding exactly to the object's internal data model. from() is a "higher-level" way to create the object that accepts more kinds of input and converts it to the object's data model. I don't mean to say this is the only way to do it, but to illustrate the design principle behind the current state.

@bijection
Copy link
Author

Thanks for the background @ptomato!

Looking through the arguments put forth in those issues, I think the above proposal still stands.

While I don't want to take sides on the new vs factory debate, if the methods currently named .from are the preferred way to create Temporal objects, it makes sense to make them first class (either as constructors, or as factories with named after the constructors) instead of relegating them to static member-factories, for the above reasons.

Re: internal data models, although I understand the appeal of having method signatures match internal data models, I think that amounts to a 'leaky abstraction' when those data models are not the most useful ones for the end user. If anything, the positional-arg methods that match the internal data models could be included as static factory methods for the rare cases where users prefer them - they could look something like Temporal.DateTime.fromNumbers(0,0,0,0,2).

@bijection bijection changed the title Proposal: Temporal.Something(...args) instead of Temporal.Something.from(...args) [Proposal] Temporal.Something(...args) instead of Temporal.Something.from(...args) Jul 24, 2020
@ptomato
Copy link
Collaborator

ptomato commented Jan 14, 2021

During the discussions when we iterated on the API based on this feedback, we decided to stay with the status quo. The split between low-level constructors and high-level from() has been a core design principle of Temporal since quite early on, and although other options seem just as valid, this is the one that we felt works for Temporal.

@ptomato ptomato closed this as completed Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants