Skip to content

array.reduce() improvements#10555

Merged
wbamberg merged 7 commits intomdn:mainfrom
hamishwillee:arrayreduce
Nov 22, 2021
Merged

array.reduce() improvements#10555
wbamberg merged 7 commits intomdn:mainfrom
hamishwillee:arrayreduce

Conversation

@hamishwillee
Copy link
Copy Markdown
Collaborator

Fixes #10524

This is not precisely a "fix" but is a slight improvement to Array.prototype.reduce() that fell out of the discussion (i.e. that issue is not actually a bug, but did make us think we could improve this).

What this does:

  • In introduction provides overview of what happens in the first iteration.
  • Splits the with/without initial value examples into separate headings and just uses arrow functions. Also uses different array values so that the things that relevant points are easier to see in the tables (i.e. the index iteration and the movement of the previousValue.

A really thorough fix might be to replace the spec inclusion in the description with hand written text. I could do that, and I think it would be good, but there are things I consider higher value at this point.

@hamishwillee hamishwillee requested a review from a team as a code owner November 16, 2021 01:45
@hamishwillee hamishwillee requested review from teoli2003 and removed request for a team November 16, 2021 01:45
@github-actions github-actions bot added the Content:JS JavaScript docs label Nov 16, 2021
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 16, 2021

Preview URLs

Flaws

None! 🎉

External URLs

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/Reduce
Title: Array.prototype.reduce()
on GitHub

No new external URLs

(this comment was updated 2021-11-22 18:12:02.257109)

Copy link
Copy Markdown
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.

Thanks @hamishwillee ! I think this is better! I found a few issues and had a couple of other suggestions.

hamishwillee and others added 2 commits November 19, 2021 12:16
Co-authored-by: wbamberg <will@bootbonnet.ca>
@hamishwillee
Copy link
Copy Markdown
Collaborator Author

Thanks. Ready for re-review!

Copy link
Copy Markdown
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.

Thanks for the updates @hamishwillee ! The description of the arguments, especially the tables, need to be updated given we have different array values now.

call being as follows:
The callback would be invoked four times, with the arguments and return values in each call being as follows:

<table class="standard-table">
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If the array is const array = [15, 16, 17, 18, 19];, then the values in this table need to be updated.


The value returned by `reduce()` would be that of the last callback
invocation (`10`).
The value returned by `reduce()` would be that of the last callback invocation (`35`).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

...and this one too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @wbamberg . Both fixed. Wish I had a good excuse.

Copy link
Copy Markdown
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.

👍 thanks Hamish!

@wbamberg wbamberg merged commit f6ebb5d into mdn:main Nov 22, 2021
@hamishwillee hamishwillee deleted the arrayreduce branch November 22, 2021 22:07
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Content:JS JavaScript docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with "Array.prototype.reduce()": first currentIndex value

2 participants