1

I've made a promise based function which crawls up a hierarchy until it reaches the top, and resolves with an object containing the structure. My only gripe with the code is that I modify variables outside the function body, meaning that it is not a pure function. I've looked into JavaScript closures, and I fully grasp trivial uses of them. But I'm struggling to figure out how/if they can help make my function pure. My attempts at making a closure so far have only overwritten the variables, not modified them. Here is the code in question using global variables:

/* I want to move these variables inside function body to purify 'getPriorRows'*/
let priorRows = {}, level = 0;

const getPriorRows = id => new Promise(resolve => {
  fetch(`/api/org/${id}`).then(result => {

    /* global varaiables are modified here */
    priorRows[level++] = result;

    if (result.parentID) resolve(getPriorRows(result.parentID));
    else resolve(priorRows);

  });
});

getPriorRows('123432').then(result => console.log(result));

Any input on the matter is greatly appreciated.

Audun Olsen
  • 597
  • 6
  • 17

2 Answers2

3

You should be able to enclose the entire function and its "external" variables in a new function:

function getPriorRows(id) {

  let priorRows = {}, level = 0;
  const getNext = id => new Promise(
     ...
  );

  return getNext(id);
}

That said, your creation of an explicit new Promise in each iteration is a Promise anti-pattern:

function getPriorRows(id) {

  let priorRows = {}, level = 0;

  const getNext = id => fetch(`/api/org/${id}`).then(result => {
    priorRows[level++] = result
    if (result.parentID) {
      return getNext(result.parentID));
    } else {
      return priorRows;
    }
  });

  return getNext(id);
}

Either way, the advantage of wrapping the state like this is that you could now have multiple calls to getPriorRows proceeding in parallel without interfering with each other.

EDIT second code edited to fix a copy&paste error with the recursion - you must call the inner function recursively, not the outer one.

Alnitak
  • 334,560
  • 70
  • 407
  • 495
3

Pass the values as arguments:

function getPriorRows(id, priorRows = {}, level = 0) {
  return fetch(`/api/org/${id}`).then(result => {
    /* global varaiables are modified here */
    priorRows[level] = result;

    if (result.parentID) return getPriorRows(result.parentID, priorRows, level+1);
    else return priorRows;
  });
}

getPriorRows('123432').then(result => console.log(result));

You can use either default parameters or a wrapper function, you don't even need a closure:

function getAll(id) { return getPriorRows(id, {}, 0); }

Also the I removed the Promise constructor antipattern.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Were you an Erlang programmer in an earlier life? ;-) – Alnitak Feb 05 '19 at 15:50
  • @Alnitak Not to my knowledge… I'm doing some Haskell from time to time, though. – Bergi Feb 05 '19 at 15:53
  • 2
    Ah, close enough. Using a function with one arity for the "public" interface that then calls the same-named function but with a different arity (with the additional "state" parameters) is a fairly common pattern in Erlang. – Alnitak Feb 05 '19 at 15:54
  • @Alnitak auxiliary/helper functions with added state variables is a common pattern in many languages – Mulan Feb 05 '19 at 20:56
  • Thank you so much. This was a very elegant solution. This approach will spare me a couple lines compared to a wrapped function. Also, thanks a lot for the heads-up about abuse of promise constructors, perfect for an async beginner like myself. – Audun Olsen Feb 06 '19 at 11:34