Elyses Enchantments tests lack coherence

Programming JavaScript
#1

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…

#2

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?
#3

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.

#4

@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
#5

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.

#6

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.

#7

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
Locomotive Engineer exercise relies on implicit Ruby behavior