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

Add section to all array methods about genericness #21137

Merged
merged 7 commits into from
Oct 7, 2022

Conversation

Josh-Cena
Copy link
Member

Description

Like #20568, this time for generic array methods.

Motivation

We mention this property on some pages but not all of them, and quite inconsistently. This may be part of mdn/mdn-community#148

Additional details

Related issues and pull requests

@Josh-Cena Josh-Cena requested a review from a team as a code owner September 28, 2022 00:49
@Josh-Cena Josh-Cena requested review from Elchi3 and removed request for a team September 28, 2022 00:49
@github-actions github-actions bot added the Content:JS JavaScript docs label Sep 28, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 28, 2022

Preview URLs (39 pages)

(this comment was updated 2022-10-07 05:30:28.944725)

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice.

The following code creates the `myFish` array-like object containing four
elements and a length parameter, then removes its last element and decrements the length
parameter.
The `pop()` method reads the `length` property of `this`. If the length is 0, `length` is set to `0` again (whereas it may be negative or `undefined` before). Otherwise, the property at `length - 1` is returned and [deleted](/en-US/docs/Web/JavaScript/Reference/Operators/delete).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I don't understand this.

If the length is 0, length is set to 0 again (whereas it may be negative or undefined before).

If length is 0, how could it have been "negative or undefined before"? Before what?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really struggle to explain this... This is explained in array/index.md:

The length property is converted to a number, truncated to an integer, and then clamped to the range between 0 and 253 - 1. NaN becomes 0, so even when length is not present or is undefined, it behaves as if it has value 0.

Maybe I should say "if the normalized length is 0"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed an improvement in 90206f2

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is better but I don't love "normalized length" on its own, because "normalize" to me can mean all kinds of things.

I think it would be better to do something like this if you had a #### Normalizing the length property in https://pr21137.content.dev.mdn.mozit.cloud/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#generic_array_methods, just before the paragraph about "The length property is converted to a number...".

You would also need something like #### Array-like objects before the paragraph starting "The term array-like object".

Then you can link "[normalized length]" to that "Normalizing the length property" fragment, and the connection should be really direct for people.

But this is just a suggestion, if you don't like it I'm happy to merge this PR as it is.

@@ -78,6 +76,28 @@ while (typeof (i = names.shift()) !== 'undefined') {
// Andrew, Edward, Paul, Chris, John
```

### Calling shift() on non-array objects

The `shift()` method reads the `length` property of `this`. If the length is 0, `length` is set to `0` again (whereas it may be negative or `undefined` before). Otherwise, the property at `0` is returned, and the rest of the properties are shifted left by one. The `length` property is decremented by one.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I don't understand this.

If the length is 0, length is set to 0 again (whereas it may be negative or undefined before).

If length is 0, how could it have been "negative or undefined before"? Before what?


The usual practice is to access {{jsxref("Array/length", "length")}} and calculate the index from that — for example, `array[array.length - 1]`. The `at()` method allows relative indexing so this can be shortened to `array.at(-1)`. More formally, when `index < 0`, `index + array.length` is accessed.

The `at()` method is [generic](/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#generic_array_methods). It only expects the `this` value to have a `length` property and integer-keyed properties.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems repetitive and redundant, given "Array methods are always generic" in https://pr21137.content.dev.mdn.mozit.cloud/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array.

I think personally we would do would be better to omit it, except when there are specific things to note about it. But I won't insist on this if you don't agree.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better if we can put it on every page—maybe one day it will no longer be the case that all array methods are generic (though I doubt it). Not many people read the Array landing page either, and it's better if we can make every page self-contained, or at least include a cross-linking (which is the main purpose here).

Co-authored-by: wbamberg <will@bootbonnet.ca>
@Josh-Cena
Copy link
Member Author

Thanks for being so careful with the examples @wbamberg You can tell I was not feeling the greatest when writing this stuff.

@Josh-Cena Josh-Cena requested a review from wbamberg September 30, 2022 01:29
@Josh-Cena
Copy link
Member Author

Added two headings; let's see if this is better.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @Josh-Cena , thank you for your patience with my pedantry :).

@wbamberg wbamberg merged commit eff56eb into mdn:main Oct 7, 2022
@Josh-Cena Josh-Cena deleted the array-generic branch October 7, 2022 21:09
@Josh-Cena
Copy link
Member Author

Thanks for all the review! 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:JS JavaScript docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants