0

Bear with me, because this takes some setting out.

Consider this trivial tree of values, for the purpose of the question / discussion.

const valuetree = [{
    item: 1,
    value: 100,
  },
  {
    item: 2,
    value: [{
      item: 3,
      value: [{
          item: 4,
          value: 200
        },
        {
          item: 5,
          value: 200
        },
        {
          item: 6,
          value: 200
        },
      ]
    }]
  },
];

Now if we wanted to write a recursive function to add up all the values, then this in itself is fairly trivial. Like so :

function sumTree(leaf) {
  let total = 0;

  if (Array.isArray(leaf)) {
    leaf.forEach(subLeaf => {
      total += sumTree(subLeaf);
    });
  } else {
    if (Array.isArray(leaf.value)) {
      total += sumTree(leaf.value);
    } else {
      total += Number(leaf.value);
    }
  }

  return total;
}

Now suppose we want to use promises to achieve the same thing?

This is what I end up with ...

async function sumTreeAsync(leaf) {
  let promises = [];

  if (Array.isArray(leaf)) {
    leaf.forEach(async subLeaf => {
      promises.push(sumTreeAsync(subLeaf));
    });
  } else {
    if (Array.isArray(leaf.value)) {
      promises.push(sumTreeAsync(leaf.value));
    } else {
      promises.push(leaf.value);
    }
  }

  return promises;
}

So what gets returned looks like this

[ Promise { [ 100 ] }, Promise { [ [Promise] ] } ]

Which makes sense, an array of 2 top level items, a promise of a literal value, and a promise that returns a nested array of promises.

So now as some function like Promise.all only deals with a flat array of promises, I would have to recurse the nested promises to arrive at the same outcome. So now I have to recurse the tree twice, which just smells so bad.

So perhaps I need to return resolved promises from the 2 cases where the subleaf is an array?

Is it even worth doing this or should recursion just be synchronous?

  • 4
    What *do* you expect as return value? What is your goal? Do you maybe want an iterator? Not sure why you want promise(s). – trincot May 19 '21 at 10:46
  • 2
    [Do not use `leaf.forEach(async subLeaf => {…})`!](https://stackoverflow.com/q/37576685/1048572) – Bergi May 19 '21 at 12:23
  • 3
    A function should in general not return an array of promises, but just call `Promise.all` itself and return a promise for an array. – Bergi May 19 '21 at 12:24
  • What I am trying to do is walk the tree and add up all the values to a total amount, in the most time efficient way. As it clearly can be concurrently processed at each level, I wanted to use async/await. But when you lump recursion and promises together things get a bit fiddly. – Charlie Benger-Stevenson May 19 '21 at 13:29
  • @Bergi: Your general point about `forEach (async (foo) => {...})` makes perfect sense. But is it really applicable here, where the function just pushes a new `Promise` into an array? There are other reasons not to bother with this technique here, but this critique doesn't seem to me to apply. Am I missing something? – Scott Sauyet May 19 '21 at 19:14
  • 1
    @ScottSauyet Yes, this was meant as a general critique, and a pointer to the canonical question about asynchronous looping. Of course, to be more specific, this function should not be `async` in the first place if it's just pushing promises into an array, and also it should be simplified to `return Promise.all(leaf.map(sumTreeAsync))`. But trincot's question stands, until we get a response from the OP I won't write an answer. – Bergi May 19 '21 at 19:24

1 Answers1

2

You're not far off. If you change your return to do a Promise .all call and .then (sum) for an obvious sum function, this would work:

const sum = (ns) => ns .reduce ((a, b) => a + b, 0)

async function sumTreeAsync(leaf) {
  let promises = [];

  if (Array.isArray(leaf)) {
    leaf.forEach(async subLeaf => {
      promises.push(sumTreeAsync(subLeaf));
    });
  } else {
    if (Array.isArray(leaf.value)) {
      promises.push(sumTreeAsync(leaf.value));
    } else {
      promises.push(leaf.value);
    }
  }

  return Promise.all(promises).then(sum);
}

const valuetree = [{item: 1, value: 100}, {item: 2, value: [{item: 3, value: [{item: 4, value: 200}, {item: 5, value: 200}, {item: 6, value: 200}]}]}];

sumTreeAsync (valuetree)
  .then (console .log)

But I would definitely write this code in a different way:

const sum = (ns) => ns .reduce ((a, b) => a + b, 0)

const sumTreeAsync = async (leaf) =>
  Promise .all (Array .isArray (leaf)
    ? leaf.map (sumTreeAsync)
    : Array .isArray (leaf .value)
      ? [sumTreeAsync (leaf .value)]
      : [leaf .value]
  ) .then (sum)

const valuetree = [{item: 1, value: 100}, {item: 2, value: [{item: 3, value: [{item: 4, value: 200}, {item: 5, value: 200}, {item: 6, value: 200}]}]}];

sumTreeAsync (valuetree)
  .then (console .log)

More than that. I would probably not write this code at all. JS is still generally a single-threaded language. So you get no concurrent calculations at all in this approach. If instead of your simple summation, you were handing things off to different workers to process, then it might make sense. As it is, this just feels like an unnecessary complication.

Scott Sauyet
  • 49,207
  • 4
  • 49
  • 103
  • Thanks Scott. As I say it was a trivial example, and the crux of the matter was about concurrency. I agree, this looks like an unnecessary complication. I know js is single threaded but I was really wondering if there were processing gains to be made from making this concurrent through the use of async/await. If instead of summing the values each value was a piece of code to be executed, and instead of a tree it was a graph with parallel routes... – Charlie Benger-Stevenson May 19 '21 at 19:27
  • It's hard to imagine any processing gains from this, only losses. But for more complex examples, and in the presence of workers, we'd have to look again. – Scott Sauyet May 19 '21 at 19:51
  • @CharlieBenger-Stevenson "*making this concurrent through the use of async/await.*" that wouldn't make it concurrent. Promises are still resolved sequentially. In order to gain concurrency, you need to hand over processing to either workers or other processes. The example code has absolutely no reason to be async as the value is there. However, it *might* make sense if the tree was asynchronously populated for example, or if the values were async (e.g., tree is built upfront but you fire off a `fetch()` for each leaf value). Or anything where you don't have all data at once. – VLAZ May 21 '21 at 06:29