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

curriculum: Need a more robust way to test for destructuring in the Use Destructuring Assignment to Assign Variables from Arrays challenge #38024

Closed
RandellDawson opened this issue Jan 7, 2020 · 10 comments
Labels
help wanted Open for all. You do not need permission to work on these. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.

Comments

@RandellDawson
Copy link
Member

RandellDawson commented Jan 7, 2020

Per this forum topic, it was discovered our test for detecting destructuring, is not very flexible

Link to challenge

For example, the following should pass as the current challenge is written.

let a = 8, b = 6;
// change code below this line
let arr = [a, b];
[b, a] = arr;

// change code above this line
console.log(a); // should be 6
console.log(b); // should be 8

My first thought was to change the instructions to state something like:

Use destructuring assignment to swap the values of a and b so that a receives the value stored in b, and b receives the value stored in a. Do not create any additional variables to solve this challenge.

While that would deal with the above code being used, it would not deal with something like:

let a = 8, b = 6;
// change code below this line
[b, a] = Array(a,b); // or [b, a] = [a].concat(b);

// change code above this line
console.log(a); // should be 6
console.log(b); // should be 8

Anyone have any thoughts on how best to change this challenge's instructions and/or test?

@RandellDawson RandellDawson added help wanted Open for all. You do not need permission to work on these. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. labels Jan 7, 2020
@moT01
Copy link
Member

moT01 commented Jan 8, 2020

In my opinion, the instructions for this challenge don't seem to line up with the description. The description shows how to destructure things from an array, the instructions ask to "destructure" and swap the values, but the seed code gives nothing to destructure. I feel like the seed code needs to start you off with an array, and the challenge should be to just destructure something from it without swapping values.

an easier fix without rewriting might be:
the first two tests check that the values for b and a are correct, which all these should pass. The third checks that destructuring was used. Perhaps the regex could be changed to just check for the [b,a] = ? instead of what it is now - Trying to think... is that something that would have to be used there. Or is there another way.

@ojeytonwilliams
Copy link
Contributor

I feel like the seed code needs to start you off with an array, and the challenge should be to just destructure something from it without swapping values.

I don't mind the swapping, since it's a useful trick to know. Giving the user an array to swap seems like a good idea, though.

How about:

Use destructuring assignment with the array, arr, to swap the values of a and b so that a receives the value stored in b, and b receives the value stored in a.

and

let a = 8, b = 6;
const arr = [a, b];
// change code below this line

// change code above this line
console.log(a); // should be 6
console.log(b); // should be 8

as the seed?

@SavvyShah
Copy link
Contributor

We could also change test like below:-

- text: You should use array destructuring to swap a and b.
    testString: assert(/\[\s*(\w)\s*,\s*(\w)\s*\]\s*=/g.test(code));

This thing matches any of the methods even [a].concat(b) and doesn't match any solutions without destructuring.

This is just if you want to be flexible with the tests as @RandellDawson says or any solutions of @moT01 or @ojeytonwilliams would work . Tell me what you guys think.

@SavvyShah
Copy link
Contributor

Have I said something wrong. I am open for criticism.

@moT01
Copy link
Member

moT01 commented Jan 13, 2020

No, nothing is wrong @ShubhamCanMakeCommit. That sounds similar to my "easier" suggestion. I would be fine fixing it like this.

@snigo
Copy link
Contributor

snigo commented Jan 26, 2020

Technically this is still destructuring:

let a = 8, b = 6;
// change code below this line
let c;
[c] = [b];
[b] = [a];
[a] = [c];
// change code above this line
console.log(a); // should be 6
console.log(b); // should be 8

I think you explicitly need to point out that a swap should be done in one operation and therefore this should fail:

let arr = [a, b];
[b, a] = arr;

@RandellDawson
Copy link
Member Author

I think you explicitly need to point out that a swap should be done in one operation and therefore this should fail:

@snigo We could specify that in the instructions. How could we test to make sure it is on one line?

@ilenia-magoni
Copy link
Contributor

the discussion here stopped midway, there is any preferred way on the direction of this?

@RandellDawson
Copy link
Member Author

I think you explicitly need to point out that a swap should be done in one operation and therefore this should fail:

@snigo Any thoughts on how best to test that it is performed on one line?

@moT01
Copy link
Member

moT01 commented Jul 20, 2021

Thank you for reporting this issue.

This is a standard message notifying you that the issue has become stale, so we are going to close it.

If you think we're wrong in closing this issue, please request for it to be reopened. Thank you and happy coding.

@moT01 moT01 closed this as completed Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Open for all. You do not need permission to work on these. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.
Projects
None yet
Development

No branches or pull requests

6 participants