Closure returning array/object exposes its lexical context: best way to tackle in JavaScript?

Question

I suppose this might be a duplicate, but I have been unable to find an explanation yet.

Here is my sample code:

const makeCalendar = () => {
  const calendar = {};

  calendar.xmas = ['December', 25];
  calendar.newYear = ['January', 1];

  return (day) => calendar[day];
}

calendar = makeCalendar();
const xmasArray = calendar('xmas');

console.log(calendar('xmas')); // [ 'December', 25 ]

xmasArray[1]++;

console.log(calendar('xmas')); // [ 'December', 26 ]

Since elements of the xmasArray are mutable, I can change the variables inside the scope of makeCalendar() and so corrupt the closure it returns. The only way I found to tackle this issue is to return an anonymous array [...calendar[day]] (instead of calendar[day]), which then blocks the access inside makeCalendar().

My questions are: is it the right way to deal with this problem? Are there better ways? Probably I don't understand correctly what's going on...

`return (day) => calendar[day];` --> replace with: `return (day) => JSON.parse(JSON.stringify(calendar))[day];` Not the best related to performances, but as long as the original array / object is not huge it should not be a deal. Using JSON.parse and JSON.stringify you're making a copy of the original object, keeping the original one safe. — briosheje, Dec 14 '18 at 08:15
Do you need the dates to be arrays? Can they be strings instead? If you absolutely need to return arrays, store the values as strings like `'December 25'` and change the function to `return day => calendar[day].split(' ');` — ic3b3rg, Dec 14 '18 at 08:20
Not exactly better, just different. By making a deep copy of the original array, the pointer you're returning (since arrays and objects are passed by **reference**) points to **another object** in memory, so there is no way to alter the original calendar object, because you're just not pointing to it. — briosheje, Dec 14 '18 at 08:20
@briosheje Thanks! I also understant it this way, just wanted to be sure. — Eugene Barsky, Dec 14 '18 at 08:22
@ic3b3rg Of course, and this is a sample code. I just want to understand the principles and to learn the best practices. — Eugene Barsky, Dec 14 '18 at 08:23
instead of returning an array you can simply return a continuation `return day => k => k(calendar[day])`. Then you apply it like `calendar('xmas') (([m, d]) => ...)` — , Dec 15 '18 at 20:40
@reify I don't fully understand your comment, but I feel that I miss something very important. Could you please write in more details, probably make an Answer? — Eugene Barsky, Dec 15 '18 at 22:26

score 1 · Answer 1 · answered Dec 14 '18 at 08:18

1

const makeCalendar = () => {
  const calendar = {};

  calendar.xmas = ['December', 25];
  calendar.newYear = ['January', 1];

  return (day) => JSON.parse(JSON.stringify(calendar))[day];
}

Not the best if the calendar object gets significantly big, but still works. In that case specifically, even JSON.parse(JSON.stringify(calendar[day])) would work.

The trick is that doing parse and stringify will create a new copy of the original object, so the original one won't be affected whatsoever. You can, of course, use other ways to clone the object, you can find more infos here: What is the most efficient way to deep clone an object in JavaScript?

fiddle: http://jsfiddle.net/briosheje/317hg6fb/

answered Dec 14 '18 at 08:18

briosheje

7,356
2
32
54

Thanks! Could you please explain, why it is better than simply returning an anonymous array? I suppose, because it's more universal? – Eugene Barsky Dec 14 '18 at 08:20
1

@EugeneBarsky I gave a brief explanation above (in the question's comments). The "anonymous array" approach is not exactly an anonymous array, but a **new array** filled with values. It's efficient, of course, but it works as long as you're spreading values in it, otherwise it won't. This way is always safe and, as long as you don't deal with huge objects, you shouldn't even see any performance issue whatosever. – briosheje Dec 14 '18 at 08:23
1

This isn't really different at all from OP's consideration of `[...calendar[day]]` – CertainPerformance Dec 14 '18 at 08:24
1

@CertainPerformance yep, it's just filling a new array with new values. I'm pretty sure there are cleverer approaches, this one feels quite safe in that case, though, and should be easy to mantain as long as the original object doesn't get too big. Is there any other approach you can suggest? You're usually great at these, from what I see from your posts. – briosheje Dec 14 '18 at 08:25

CertainPerformance · Accepted Answer · 2018-12-14T08:28:06.733

1

One option would be to use Object.freeze to forbid assignment to any items in the array:

const makeCalendar = () => {
  const calendar = {
    xmas: ['December', 25],
    newYear: ['January', 1]
  };
  Object.values(calendar).forEach(arr => Object.freeze(arr));
  return (day) => calendar[day];
}

calendar = makeCalendar();
const xmasArray = calendar('xmas');

console.log(calendar('xmas'));

xmasArray[1]++;

console.log(calendar('xmas'));

Note that such attempted assignment will throw an error in strict mode:

Uncaught TypeError: Cannot assign to read only property '1' of object '[object Array]

edited Dec 14 '18 at 08:28

answered Dec 14 '18 at 08:26

CertainPerformance

356,069
52
309
320

Hoh, clever, indeed. Never seen Object.freeze being used before, though I see it's well supported. – briosheje Dec 14 '18 at 08:27

Closure returning array/object exposes its lexical context: best way to tackle in JavaScript?

2 Answers2