The enchantment.js file is documented, specifying that the function should return an array of numbers. Some of the tests don’t use the returned value, but consider that the initial array has been modified. Considering that the other tests are using the returned value, why not these ones ? It makes the test fail if you don’t mutate the array for these specific tests, while you don’t have to mutate the array in the other ones…
Hi Dylan,
In threads like this it often helps to be very explicit. For example,
- Can you quote the relevant bits of the instructions?
- Can you quote/annotate or otherwise single out the relevant tests?
- Can you give a solution that should pass but does not?
Example:
/**
* Insert newCard at the end of the cards array
*
* @param {number[]} cards
* @param {number} newCard
*
* @returns {number[]} the cards with the newCard applied
*/
export function insertItemAtTop(cards, newCard) {
// The test expects the new card to be added to the end of the array by mutating the original array~
cards.push(newCard);
return cards;
/* Without mutating the original array, the following would be the correct implementation:
return [...cards, newCard];
*/
}
I think that since the function is returning the new array, and not being void, this value should be used, and the original array should not have to be mutated to pass the test. Some test would call these functions twice without assigning it.
I mean, both solution should be valid.
@Dyrits,
I appreciate your courage to question the tests when you are still new here. Having solved the exercise a while before, I went back to see which test is expecting a mutation of the input array and whether it is stated in the exercise instructions. I could not find anything in the instructions to that effect.
Here is the only test that would fail if you don’t mutate the incoming array…
test('adding multiple cards to the stack at the top', () => {
const stack = [1];
insertItemAtTop(stack, 5);
insertItemAtTop(stack, 9);
const expected = [1, 5, 9];
expect(stack).toStrictEqual(expected);
});
The reason is clear - two calls are made successively. This seems acceptable to me. How do you expect to handle such a scenario?
1 Like
I guess I’m wrong about it and every function should mutate the array since it says “Return the adjusted stack”.
It’s just about coherence in the exercise since a learner could get some of the tests succeed by not mutating the array in some of the functions, and fail in others. It’s not really intuitive. Either every test should test the returned value, either they should test the modified stack, or both.
Unfortunately, that is not true for even the standard library of JavaScript.
On your PR I responded the following:
The forum has a lengthy post on why “It should not return something if you mutate it” isn’t necessarily a rule we should follow: Discouraged code practice found in Elyses Enchantments - #8 by SleeplessByte.
Feel free to continue discussion here if you wish.
DJ and I chatted and think the easiest way to solve this confusion is to just be more explicit in the instructions. I’ve put a PR together.
1 Like
I recently reopened the exact same issue so it seems to me the PR is not completely solving it.
I read the related conversations so (I think) I get why you want to keep mutability under the carpet but for learner like me that rely on “task description” to do them step by step it’s still confusing.
What if the comments were updated to specify what is set in the admonition ?
something like “do blabla in the cards array and then return it”.
In this particular case I wouldn’t have much of an issue with the JSDoc being updated (I think that’s what you’re suggesting?) but i dont feel much for supporting of that means skipping to read the instructions.
Not because I think it’s bad to skip them, but because it’s a slippery slope where people will request more and more to live in the comments up to the point where the entire instructions are obsolete.
As the docstring is already there you already took that route ;-)
TBH I think this is not as easy to understand as you may think. The assumption is “my learners are complete beginers so they don’t even know the concept of mutation”. On my side, I started coding during my studies for HPC so everything was about pointers either in C or in Fortran. To me it was a major concern and understanding that it’s no longer a main preocupation in javascript was a huge gap (that and the absence of static casting).
Mutation is a key concept and it leads to many issues (that you mentioned in the long answer). So if you don’t want to change the docstring, be crystal clear in the instruction and don’t rely on the admonition. I would write something like: “modify the card array to add a newcard at the end”.
after writting it I start asking myself, it may be a bad idea to be shy from the start, what about “mutate the card array to add a new card at the end” ? It will generate as much curiosity and it avoids hiding the elefant under the carpet.
We will need a concept exercise about mutation if we want to go that route. Mutation isn’t a concept that exists in all programming languages, or extend the instructions a lot, except that you indicated that some people just skip those, so I don’t understand how any of what you’re suggesting would help.
However, your comment about the route from C/Fortran to JavaScript is interesting to me! Would you want to share more about that? It can greatly help me and others shape the content more so people like yourself are better aided. Eventually I do want to overhaul the docs (introduction and instructions) and would want to take this information into account as well - at that moment it would also be fine to explain students about “mutability” of certain Array methods (without having to use that word).
after writting it I start asking myself, it may be a bad idea to be shy from the start, what about “mutate the card array to add a new card at the end” ? It will generate as much curiosity and it avoids hiding the elefant under the carpet.
I rather say “add a new card at the end of the deck” and then remove the return completely, explaining in the instructions that it’s supposed to modify the deck instead of returning a new one. That probably solves your issue too?
it does indeed.
However, your comment about the route from C/Fortran to JavaScript is interesting to me! Would you want to share more about that?
How can I participate ? a call ? Private messages ?