Missing test in 'Coordinate Transformation' exercise

Programming JavaScript
1

Hi everyone! I stumbled on a (probably) missing test in ‘Coordinate Transformation’ exercise in the JavaScript track.

Here is my solution to the last task, ‘Save the results of functions’, which passed all the tests:

export function memoizeTransform(f) {
  let savedX, savedY, savedResult;
  return function memoize (x, y) {
    if (x != savedX && y != savedY) {
      savedX = x;
      savedY = y;
      savedResult = f(x, y);
    }
    return savedResult;
  };
}

Later, my mentor @mayan164 explained to me that there is an error in my function’s logic:

Your last function is great, except that it should be a || instead of a &&. When only one of the inputs changes, the result should be recomputed. Example: with f being the sum of x and y, a first call with x=1, y=1 has a different result than a second call with x=1, y=2. Using a && leads to keeping the same result because x === savedX. The tests are not always sufficiently complete to verify all cases, that’s why your solution passed all tests.

It would be helpful to have a test that checks for only one change in the inputs, to help other people who might make the same mistake. (Or maybe my mistake was so dumb that no one will ever make it again, then you can ignore this post :sweat_smile:)

4 Likes
2

After looking at the tests for this exercise on GitHub, I propose adding one new line to the should return different results for different inputs test:

test('should return different results for different inputs', () => {
  const memoizedTranslate = memoizeTransform(translate2d(1, 2));
  expect(memoizedTranslate(2, 2)).toEqual([3, 4]);
  expect(memoizedTranslate(2, 1)).toEqual([3, 3]);  // Proposed new test
  expect(memoizedTranslate(6, 6)).toEqual([7, 8]);
});
2 Likes
3

This would be a good addition. We’ll take a PR for this. You can get some reputation if your Exercism and GitHub account are linked.

We can help you if you want to contribute but don’t know how to.

2 Likes
4

Great! Here’s the PR: Add a new test to 'Coordinate Transformation' exercise by mytiador · Pull Request #2795 · exercism/javascript · GitHub

2 Likes
5

Thank you, it has been merged!

2 Likes