Skip to content

Clarification of array example #2491

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

Closed
wants to merge 4 commits into from
Closed

Conversation

PierBover
Copy link

It was a bit confusing that the value added was the length of the array. I added a value var to make the example more obvious to all audiences.

So that it is more consistent with the tutorial text.
So that it is more consistent with the tutorial text.
@codecov-io
Copy link

codecov-io commented Apr 22, 2019

Codecov Report

Merging #2491 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2491   +/-   ##
=======================================
  Coverage   91.83%   91.83%           
=======================================
  Files           1        1           
  Lines          49       49           
=======================================
  Hits           45       45           
  Misses          4        4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58531cb...428c737. Read the comment docs.

@Rich-Harris
Copy link
Member

Thanks, but this wouldn't actually work, because you're not passing a value to addNumber — you're passing an event object instead:

1 + 2 + 3 + 4 + [object MouseEvent] + [object MouseEvent] = 10[object MouseEvent][object MouseEvent]

The way to fix that would be to put the same logic inline...

-<button on:click={addNumber}>
+<button on:click={() => addNumber(numbers.length + 1)}>
  Add a number
</button> 

...which puts us back where we started. The only alternative would be to keep track of n separately, like this...

let n = 1;
let numbers = [n++, n++, n++, n++];

function addNumber() {
  numbers = [...numbers, n++];
}

...but that doesn't seem clearer to me. I don't think it's a stretch to expect folks to understand what numbers.length is.

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

Successfully merging this pull request may close these issues.

3 participants