Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Names of functions #10

Closed
rricard opened this issue Apr 2, 2021 · 53 comments · Fixed by #50
Closed

Names of functions #10

rricard opened this issue Apr 2, 2021 · 53 comments · Fixed by #50
Labels
question Further information is requested
Milestone

Comments

@rricard
Copy link
Member

rricard commented Apr 2, 2021

See tc39/proposal-record-tuple#121, but we might want to discuss naming before commiting to this.

@rricard rricard added the question Further information is requested label Apr 2, 2021
@acutmore
Copy link
Collaborator

acutmore commented Apr 15, 2021

Some data that could feed into this discussion.

Kotlin has
list.sort() (mutating) docs
list.sorted() (non-mutating) docs

Java has
Arrays.sort(array) (mutating) docs
stream.sorted() (non-mutating) docs

Python has
array.sort() (mutating) docs
sorted(array) (non-mutating) docs

Swift has
collection.sort() (mutating) docs
collection.sorted() (non-mutating) docs

@bmeck
Copy link
Member

bmeck commented Apr 20, 2021

Just to be clear, using past participles makes conversation hard: E.G. "I popped the array." has a fairly easy to understand meaning currently but with different phrasing with past participles it could suddenly mean either the mutating or non-mutating method, which potentially even have differing return expectations.

@arxpoetica
Copy link

arxpoetica commented Apr 20, 2021

Definitely prefer something that reads different enough for eye scanning purposes:

Tuple.prototype.copyPush (vs. .pushed)
Tuple.prototype.copyPop (vs. .popped)
Tuple.prototype.copyReverse (vs. .reversed)

vs. just past tense adding ed

@acutmore
Copy link
Collaborator

From an autocomplete perspective it looks like having a common prefix instead of a suffix will not impact discoverability 👍

screenshot showing autocomplete for both reverse and copyReverse after typing in list.reverse

@rricard
Copy link
Member Author

rricard commented May 6, 2021

We will introduce with<PastTense> as the new naming scheme as described in #26. This choice can be discussed here.

@rauschma
Copy link

I like the new scheme! Maybe the past tense isn’t even needed:

  • .withFill
  • .withCopyWithin
  • .withPop
  • .withPush
  • .withReverse
  • .withShift
  • .withSort
  • .withSplice
  • .withUnshift
  • .withAt

Upside: Easier to derive the names (which sound more familiar). Downside: Doesn’t map as well to natural language.

@ljharb
Copy link
Member

ljharb commented May 20, 2021

The downside there seems to conflict with the upside - since being closer to natural language makes it easier to derive the names.

@rauschma
Copy link

rauschma commented May 20, 2021

Without past tense, the rules are simpler – just prefix “with”.

With past tense, you have to look for a verb (which may or may not be there) and then change it to past tense.

I’m also not completely sure that the past tense is really closer to natural language.

@acutmore
Copy link
Collaborator

acutmore commented May 21, 2021

Something else to discuss: If we do go for the withAt(index, value) method should the arguments be flipped to withAt(value, index)? The rationale being folk could remember the arg order with the phrase "with (value) At (index)"

@zloirock
Copy link
Contributor

zloirock commented May 21, 2021

@acutmore IIRC all current methods use (key, value). For example, defineProperty, __define[GS]etter__, Reflect.set. Index is a key.

@rricard
Copy link
Member Author

rricard commented May 21, 2021

Yea, I think we should more change withAt to something else than the other way around if we want to address this

@acutmore
Copy link
Collaborator

One idea I had was withReplaced(index, value) - this may also make it clearer that the index must fall within the existing length, unlike direct index assignment.

@Alhadis
Copy link

Alhadis commented Jun 2, 2021

IMHO, this is a mistake. Parity with a deliberately-immutable data structure (Tuples) isn't worth the confusing and inconsistent plethora of new methods. Here's our current list of array-returning verbs:

Method Modifies receiver?
Array.prototype.concat()
Array.prototype.copyWithin()
Array.prototype.fill()
Array.prototype.filter()
Array.prototype.flat()
Array.prototype.flatMap()
Array.prototype.map()
Array.prototype.pop()
Array.prototype.push()
Array.prototype.reverse()
Array.prototype.shift()
Array.prototype.slice()
Array.prototype.sort()
Array.prototype.splice()
Array.prototype.unshift()

You really want to add another ~10 methods to that list just to be consistent with an unrelated but superficially-similar object type? I'd rather stick with the familiar .slice().avoidModifyingOriginal() idiom to operate upon a copy of an array I don't want modified. This, combined with Array.from(foo) or even just [...foo], pretty much covers every use-case I can think of which this proposal advertises (even with frozen arrays).

This proposal notably makes it easier to write code able to deal with Arrays and Tuples interchangeably.

This strikes me as a non-argument. Sure, they're both iterable and array-ish, but so are TypedArrays. A Uint8Array doesn't have .pop() or .push() methods, because it's fixed-length. It does, however, have methods to construct another of its kind via .map() or .filter(). Such a discrepancy won't look out-of-place on Tuple.prototype, either. Conversely, a bunch of (really misleading) methods on Array.prototype isn't adding anything other than autocomplete pollution.

Moreover, how many users will need to construct tuples from other tuples often enough to justify the existence of a .pop() (or .popped(), .aufgetaucht(), whatever) method? Be honest.

@acutmore
Copy link
Collaborator

acutmore commented Jun 2, 2021

Hi @Alhadis, this issue is focused more on the naming of the methods. #27 is for discussing reducing the number of new methods.

@Alhadis
Copy link

Alhadis commented Jun 2, 2021

I must've paraphrased out the part of my comment that explicitly addresses the naming. Use methods named after their equivalent operations on Array.prototype. Developers will know what Tuple.prototype.fill does from the name alone—the fact that tuples are immutable structures by definition makes the "returning a copy" part of the methods implicit.

As for methods that don't sanely carry across to tuples (.pop(), .shift(), et al), I'd leave 'em out, honestly. If you find Tuple.from([...otherTuple, newValues]) too verbose, use different verbs:

Array.prototype.unshift => Tuple.prototype.prepend
Array.prototype.push    => Tuple.prototype.append
Array.prototype.pop     => Tuple.prototype.slice(-1)
Array.prototype.shift   => Tuple.prototype.slice(1)

I doubt the last two operations will be common enough to warrant their own methods.

@papb
Copy link

papb commented Jun 22, 2021

Quoting myself from the other issue:

My personal favorites for now are the .copy* names, such as .copyPush, .copyReverse, etc. What can I do to improve the odds of these names being chosen?

(about one year later, these are still my favorites)

@papb
Copy link

papb commented Jun 22, 2021

@Alhadis prepend and append still carry the idea of mutation to me.

@Alhadis
Copy link

Alhadis commented Jun 22, 2021

@papb What are your thoughts on sort and reverse, then?

@papb
Copy link

papb commented Jun 23, 2021

@papb What are your thoughts on sort and reverse, then?

They also carry the idea of mutation to me. For the immutable counterparts, I would suggest .copySort() and .copyReverse(). Is this what you asked? Sorry if I misunderstood.

@omeid
Copy link

omeid commented Jun 30, 2021

I think going with the simple past tense and consistent with other languages is a not a bad idea at all.

I am not convinced that scanning is an issue, once you're familiar with these methods it is not hard to look out for them since you will just scan for yyyy.XX__d(....) vs yyyy.XX__[^d](....) .

And I think it reads naturally as well.

const y = x.sorted(fn) => const y is is equal to y sorted using fn
const p = q.spliced(i, t, u1,... un) => p is equal to q spliced at index i for t elements and replaced with u1, u2, un.

To me,
const y = x.withSorted(fn) reads like y is equal to x with sorting configured to fn or something to that effect.
Because with suggests that it combines x somehow with a sorting fn, rather than apply it.

@OmgImAlexis
Copy link

OmgImAlexis commented Jul 30, 2021

Personally if I saw "with" I'd expect to need to pass a function not that it was in past tense.

array.sort() I expect this to mutate, it's sorting the original array.
array.sorted() I expect this not to mutate but instead return a new array.

This reads a lot better than adding "with" to the start.

@mohsen1
Copy link

mohsen1 commented Aug 1, 2021

Maybe because I'm not a native English speaker I had this impression but with* didn't convey the meaning of those functions to me. .sorted does.

@papb
Copy link

papb commented Aug 2, 2021

array.sorted() I expect this not to mutate but instead return a new array.

@OmgImAlexis I don't... I expect array.sorted() to do exactly the same as array.sort(). That's why I think a clearer name would be beneficial.

@jakehamilton
Copy link

I think it's important for these names to be familiar coming from the mutating versions while also making it clear there is a new copy of the array being created. Since I don't think it's been mentioned, I'd like to suggest clone* as a pattern for these functions. For example: cloneSorted, cloneReversed

Do note that this suggestion does not make a distinction between things like cloneSorted and cloneSort, only that the function name begins with "clone".

@phryneas
Copy link
Member

Another naming scheme that would make the purpose of these functions stand out more might be a prefix of immutable, or short immut.

Just throwing that one in the ring :)

@dkns
Copy link

dkns commented Aug 31, 2021

As a non-native english speaker 'withX' is hard to intuitively understandp; much harder than 'copyX' or 'toX'.

@timoxley
Copy link

timoxley commented Aug 31, 2021

Putting a vote in for toX instead of withX.

toX at least has some precedent in toString, toArray. They're changing types but it's unambiguous and clear that you're going to get a new object.

The closest thing to withX is probably copyWithin? and the within part actually helps to communicate that the method mutates, and yet here the with is trying to communicate that it doesn't mutate.

within is clearly a different word to with but I think it's similar enough in meaning and appearance to make you have to pause for a moment to remember whether "with" means mutating or not mutating. with vs within is almost a contronym. IMO contronyms don't belong in programming languages.

Like many people I struggled for a long time to remember which was which with splice vs slice, so IMO it'd be good to avoid doing that to people again.

@WebReflection
Copy link

Agreeing with this comment, and also with either as or to as prefix, where both do a better job than using with.

E.G. "I popped the array." has a fairly easy to understand meaning

And so does I have assigned the tuple as popped one.

@Rich-Harris
Copy link

Except in contrived cases, 'with' always implies the addition of something. It's therefore a confusing choice for methods that remove elements — withPopped suggests we're... adding the popped element? Perhaps this is the rare case where a native English speaker is at a disadvantage in programming, but my brain would get tied in knots trying to understand the return value of these methods.

to is a better prefix, as has been noted, but it forces the use of the past tense which I think is unfortunate on two grounds. Firstly, it moves the focus to the element rather than the array. (You don't pop an array, you pop an element from the array, or at least that's my understanding of the semantics.) toPopped to my mind is ambiguous — we could be changing the array to the array minus the popped element, or we could be converting from the array to the popped element.

Secondly, it introduces an asymmetry — the past tense of splice adds a d, the past tense of sort adds an ed, the past tense of pop adds ped. Converting between mutative and non-mutative method names is therefore not a mechanical process, and it's honestly just less aesthetically pleasing.

I propose the following scheme:

  • Array.prototype.postFill(value, start, end) -> Array
  • Array.prototype.postCopyWithin(copiedTarget, start, end) -> Array
  • Array.prototype.postPop() -> Array
  • Array.prototype.postPush(values...) -> Array
  • Array.prototype.postReverse() -> Array
  • Array.prototype.postShift() -> Array
  • Array.prototype.postSort(compareFn) -> Array
  • Array.prototype.postSplice(start, deleteCount, ...items) -> Array
  • Array.prototype.postUnshift(...values) -> Array
  • Array.prototype.postUpdate(index, value) -> Array

The odd one out here is withAt -> postUpdate, since postAt doesn't make any more sense than withAt.

@mjr
Copy link

mjr commented Aug 31, 2021

Wouldn't something like python be a good option?

Python

array.sort() # (mutating)
sorted(array) # (non-mutating)

array.reverse() # (mutating)
reversed(array) # (non-mutating)

Javascript

array.sort() // (mutating)
Array.sorted(array) // (non-mutating)

array.reverse() // (mutating)
Array.reversed(array) // (non-mutating)

@Rich-Harris
Copy link

Array.sorted(x) is 15 characters, x.toSorted() or x.postSort() is 12. What do you gain in return for the extra characters?

@mjr
Copy link

mjr commented Aug 31, 2021

There is no gain in this regard, i just suggested it to see what you think about this option.

@OrenMe
Copy link

OrenMe commented Sep 1, 2021

Is it possible to do the same as it was done with addEventListener and add an options object to the method that will allow to specify the operation type of it if it is mutating or not? I know it will change the response according to mode but this can be expected with regards of setting the option object.
This will also allow future expansion of features to the method like maybe returning sealed object or other actions and response types.

@acutmore
Copy link
Collaborator

acutmore commented Sep 1, 2021

array.sort() // (mutating)
Array.sorted(array) // (non-mutating)

array.reverse() // (mutating)
Array.reversed(array) // (non-mutating)

Thanks for the suggestion. One of the goals of this proposal is to have the same method names across Array/TypedArray and Tuple. So list.nonMutatingSort() would work regardless of the type of list. This wouldn't be achieved with static type specific methods on the separate constructors.

@lightmare
Copy link

lightmare commented Sep 9, 2021

For me

  • withXY doesn't evoke the intended meaning at all.
  • toXY is better, but to_Verb_ is a bit confusing (both toString and toJSON are to_Noun_).
  • copyXY is clear.

withAt is a bad name regardless of the prefix. The method .at is already non-mutating, and doesn't throw when index is out of range. Better pick a different name. Or drop this one, copySpliced(index, 1, value) can do the job. In a scenario where withAt would be used a lot, the Array could likely be replaced with Tuple, anyway.

Naming aside, I don't see the benefit of putting __Sorted() and __Reversed() on the prototype. Depending on the chosen prefix, all it would do is save 2-4 characters over a.slice().sort() and the like.

I mean, if saving 2-4 characters is considered worth it, then Array.sorted(a) would save 5 characters over Array.from(a).sort(), and would be more useful, as it would work with any iterable or array-like object, unlike [...a].sort() or a.copySorted().

Out of the proposed methods, only a.copySpliced() seems useful to me, but many of its use cases could be covered by proper slicing syntax, which JS currently lacks.

@WebReflection
Copy link

WebReflection commented Sep 9, 2021

So list.nonMutatingSort() would work regardless of the type of list. This wouldn't be achieved with static type specific methods on the separate constructors.

that's true for Array.from and derivatives too though, yet it makes it clear the intent to create a new collection, given a specific input, and a specific constructor to use for the new collection.

I actually think the Python ish suggestion wins over all others .. the Array.prototype is stuffed already, and methods like copySorted(fn) feel lame, compared to Array.sort(arr, fn), or even Array.sorted(...).

imho, the requirements are clear, the way forward looks already overly confusing to me.

@AnthonyLenglet
Copy link

Putting in a vote for to here, it is definitely better than with, since its already used to convey a transformation without modification in other cases like toString and toJSON

only problem is <array>.toAt, which just looks really odd and isn't the most explicit, not sure how that could be handled

also, a bit about the proposal of the word copy, it actually makes the at problem even worse

const array = [1, 5, 10];
const newArray = array.copyAt(1, 2);
console.log(newArray); // one could think this should return [1, 10, 10], but this is actually [1, 2, 10]

@acutmore
Copy link
Collaborator

Thanks for your input @AnthonyLenglet.

I think it can be an option for the withAt functionality to have a name that does not follow the same pattern as the other 3 new methods. The other methods are all non-mutating equivalents of existing methods, whereas withAt can be viewed as the non mutating form of the assignment operation list[index] = value.

@zloirock
Copy link
Contributor

@rricard @acutmore hey, do you plan to change names before the TC39 meeting? It seems that almost no one likes with, but to is not so bad.

@acutmore
Copy link
Collaborator

Hi @zloirock , the key goal of the next Tc39 meeting for us is to get volunteers to review the spec. We will mention that the current names are still very much tbc. But I don’t think we need to update the documents until the new names have been agreed.

@sarahghp
Copy link

Chiming in as big fan of the to column in the latest slides.

One thing I was confused about was at, since Array.prototype.at merely returns a value — using a plain with distinguishes the function well.

@OmgImAlexis

This comment has been minimized.

@acutmore
Copy link
Collaborator

acutmore commented Oct 28, 2021

Chiming in as big fan of the to column in the latest slides.

One thing I was confused about was at, since Array.prototype.at merely returns a value — using a plain with distinguishes the function well.

Thanks for chiming in @sarahghp!

For reference the “to” column presented at tc39 this week had:

  • .with(index, newValue)
  • .toReversed()
  • .toSorted(compareFn)
  • .toSpliced(index, deleteCount, ...valuesToInsert)

@zloirock
Copy link
Contributor

Since the most preferred to- prefix schema, I think that in core-js I'll publish polyfills for experimental usage with this naming, zloirock/core-js#923.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.