Proposal-temporal: TODO: math functions

[mj1856]

We need to decide on how we want our math functions to look. Options are basically addMonths(3) or add(3, 'months'). We could have subtract functions also, or just allow negative numbers, or both.

[maggiepint]

I've always personally been for .add('months', 3). The functional crowd likes their params in that order. I like the code flexibility you get from making unit a parameter. Of course we then have to address what those constants are.

[icambron]

I agree with Maggie that letting it be a parameter is an important feature, and in fact lets you do this anyway:

addMonths = (t, m) => t.add(m, 'months');

In fact, AFAIK the reason the functional crowd likes our params in that order is that it lets us use partials to define addMonths locally. But Javascript doesn't have a convenient way to make partials on methods, so it seems like a minor concern in this context.

Separate point: I think it should be plus not add. "Add" seems to carry more this-probably-mutates-my-object baggage.

[phueper]

i’d prefer addMonths() i think it reduces the risk for errors, helps e.g. support by IDEs for code completion, nicer to document separate methods...
e.g. what would we expect to happen when Local/PlainDate.add() would be called with add('hours', 3) how do we add hours to a Date ? throw an error? but then again add('hours', 24) might be possible... imho it is much clearer to have separate methods for the allowed types to add

[domenic]

Strong +1 for proper methods. Adding months is a different operation than adding days and should not be lumped together.

If people want something dynamic they can do that the same as they would for any other class with multiple methods where you're not sure ahead of time which to call: d['add' + type](5)

[domenic]

Something like d.plus({ months, days, ... }) could work though, echoing the setter discussion.

[icambron]

I do like the {} option, partially for the symmetry and partially because it's more powerful; you can do multiple units at a time. It occurs to me that I actually implemented this in my toy library here though I didn't emphasize it much.

[maggiepint]

For what it's worth, I'm oddly happy with the whole {} thing too. Maybe we've managed to decide this?

On the subject of math, should the types differentiate?
In some worlds, the Instant types ONLY do timeline math (hours and less), while the plain types ONLY do calendar math (Days and more). This is actually a very logically sound thing to do, as it avoids assumptions about days being 24 hours, etc.

[phueper]

I think they should differentiate, but depending on type and what they represent. PlainTime and PlainDateTime should be able to do both timeline and calendar math, while PlainDate probably only should be able to do calendar calculations... I'm unsure about Instant though.

[phueper]

No, wait... PlainTime only should be able to do timeline math... No calendar math.

[maggiepint]

Yeah. That.

On Jul 9, 2017 9:56 PM, "Pattrick Hueper" [e-mail] wrote:

No, wait... PlainTime only should be able to do timeline math... No
calendar math.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/maggiepint/proposal-temporal/issues/23#issuecomment-314004822,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFxi0icAFxRNtGmjQIhToX4eol9n9btIks5sMa8WgaJpZM4ORoXm
.

[keithamus]

With a method like d.plus({ months, days, ... }) - the order of addition can be specced internally to the function which can prevent some issues such as:

d = new Date(2017, 4, 30) // May 30
d.addMonth(1).addDay(1) // July 1
// vs
d = new Date(2017, 4, 30) // May 30
d.addDay(1).addMonth(1) // June 30

[maggiepint]

@keithamus I actually wrote the ordering feature into moment's object based set. Hadn't thought about that in a while. IMO that's a strong argument for the object based math and set.

[mj1856]

@phueper - Note that math on PlainDateTime would be timeline math as its usually understood, but math on PlainTime would be clockwise math. For example:

let t1 = new PlainTime(23, 59);    // 23:59
let t2 = t1.plus({minutes: 1});    // 00:00

[mj1856]

Oooh... I think I'm seeing the appeal of plus with an object (like my last comment). Though one wonders about mixed positives and negatives in the same object. I suppose it's not really that much of a concern on time, but on date it might be confusing.

let d1 = new PlainDate(2017, 3, 30);        // 2017-03-30
let d2 = d1.plus({ months: -1, days: 1 });  // 2017-03-01 or 2017-02-28 ?

I guess as long as we define the order that the options are applied, it would be fine. I would pick largest-to-smallest, so 2017-03-01 in the above example. Everyone like that?

[gilmoreorless]

I'm not a fan of parameters that only accept specific strings (e.g. .add('months', 3)), so +1 for the options object and/or separate methods.

Both have their pros/cons. Separate methods are good for autocompletion in IDEs (as mentioned by @phueper) and also in browser consoles (since console output has been discussed in #25).

An options object is more flexible though, and as a consumer of the API I'd rather write d.plus({ years: 1, days: 1 }) than d.plusYears(1).plusDays(1). (As a side note, would the chained API end up creating memory overhead as intermediate objects are created and discarded? I remember this being a potential problem in the immutable proposal for moment.)

Defining the order operations as largest-to-smallest makes sense, partly as it's easy to explain: Just read left-to-right on an ISO 8601 string.

[phueper]

SInce both seem to have pros/cons and both probably have people liking them better, couldn’t both be made available?
And have the user decide which one to use? plusYears(...) could basically be an alias for plus({ years }) ... some people might like the chaining approach better and think it is more readable (and have the ability to specify the order explicitly) at the cost of intermediate objects created, while some like the options object better? This way also minusXXX() could be provided as an alias for the plus() method using negative option params as @mj1856 described above?

Defining the order largest-to-smallest seemst to make most sense to me, too.

[icambron]

@mj1856 Largest to smallest seems definitely right. Note that it's not clear to me that Moment actually does this (e.g. here), so maybe some more thought is required.

@phueper Less is more. Adding a bunch of aliases seems unwieldy, and it's so easy to do in userspace that it seems unnecessary.

[maggiepint]

@icambron, it only does it with object based set (sets largest units first). We could stand to implement it everywhere.

[mj1856]

Agreed on the less-is-more mentality. While we certainly can create more than one way to do things, that doesn't necessarily help. Indeed, on #32, we're pretty much set on giving up factories in favor of just the constructors. Same kind of thinking here.

[maggiepint]

So it sounds like we have general consensus on object-based operations?

If so, we need to think through how we want to handle unit constants. Are they just strings? Do we want to provide them on the temporal namespace?

[domenic]

In JS the pattern is just strings, yeah. Precedence in both ES and on the web. No need to duplicate them as CONSTANTS.

[mj1856]

I'm confused... there are no unit strings if we go with the object-based operations.

[mj1856]

Sure. And agreed, they don't need to be duplicated as constants. Unless everywhere we use an options object pattern we define constants for the object keys, which would probably be overkill.

[mj1856]

Wow. So we actually seem to have consensus! Anyone want to send a PR? 😄

[icambron]

To make Matt not look crazy, I'll add that I answered his question "the object keys" and then deleted because I thought maybe that's not, in fact, what everyone else meant

[maggiepint]

It was what everyone else meant. But don't worry, it's commonplace for Matt to look crazy.

[icambron]

In that case, strong agree for non-constants. Pre-ES6, you can't create object literals with non-literal keys, so it would be a big pain to use constants:

var barry = temporal.createPlainTime();
var units = {};
units[temporal.SECONDS] = 23;
var otherBarry = barry.plus(units);

which is why I suspect nobody does that.

[mj1856]

Since this seems set, I'm closing this item. Thanks folks!