0

(* I modified my initial question... *)

I have an asynchronous function calculate which re-calculates a Excel workbook and prints how long it takes.

Then, I want to make the function to return the calculation time, so that I could record it. It regards making an asynchronous function return a value. I read several threads, and write the following 2 ways, which work:

function calculate1 (mode, fn) {
    return Excel.run(function (ctx) {
        ctx.workbook.application.calculate(mode);
        var before = performance.now();
        return ctx.sync().then(function() {
            var after = performance.now();
            var t = after - before;
            document.getElementById("b").value += 'inside: ' + t + '\n'; 
            fn(t);
        })
    })
}

function calculate2 (mode) {
    return new Promise(function (resolve, reject) {
        return Excel.run(function (ctx) {
            ctx.workbook.application.calculate(mode);
            var before = performance.now();
            return ctx.sync().then(function() {
                var after = performance.now();
                var t = after - before;
                document.getElementById("b").value += 'inside: ' + t + '\n';
                resolve(t); })
            })
    })
}

Here is the test:

function test () {
    var a = [];
    var pm = new OfficeExtension.Promise(function(resolve, reject) { resolve (null); });

    pm
        .then(function() { return calculate1('FullRebuild', function (t) {
            a.push(t); }); })
        .then(function() { return calculate1('FullRebuild', function (t) {
            a.push(t); }); })
        .then(function() { return calculate2('FullRebuild').then(function (result) {
            a.push(result); }); })
        .then(function() { return calculate2('FullRebuild').then(function (result) {
            a.push(result); }); })
        .then(function() {
            document.getElementById("b").value += a.toString() + '\n'; });
}

I guess calculate1 uses callback, while calculate2 uses promise. Could anyone tell me which way is better?

Additionally, is fn(t) (resp., resolve(t)) in the right place, or should I wrap it in another then?

PS: Excel.run and ctx.sync() are functions of JavaScript API for Office; they both return a promise.

SoftTimur
  • 5,630
  • 38
  • 140
  • 292
  • 2
    Take a look at this http://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it. You are mixing callback and promises. I would suggest let calculate resolve `t`, which is what you want. – Deepak Puthraya Jun 21 '16 at 07:11
  • My initial way by callback was not wrong (the error message was due to something else). Please see my updates... – SoftTimur Jun 21 '16 at 07:54
  • The one you like more is "better". – zerkms Jun 21 '16 at 07:55
  • I mean, isn't there anything redundant in one way or the other? Moreover, are the two ways fundamentally the same or different? – SoftTimur Jun 21 '16 at 07:58

2 Answers2

1

calculate1() is a bad mixture of promises and callbacks, and calculate2() is exhibiting the Explicit Promise Construction Antipattern.

Here is how you can correctly use promises without any extra waste:

function calculate1 (mode) {
    return Excel.run(function (ctx) {
        ctx.workbook.application.calculate(mode);
        var before = performance.now();

        return ctx.sync().then(function() {
            var after = performance.now();
            var t = after - before;
            document.getElementById("b").value += 'inside: ' + t + '\n'; 

            return t;
        });
    });
}

And then you can consume it like this. Note that it uses considerably fewer functions than your attempt:

return calculate1('FullRebuild')
    .then(function(res) { 
        a.push(res);
        return calculate1('FullRebuild'); 
     })
    .then(function(res) { 
        a.push(res);
        return calculate1('FullRebuild'); 
     })
    .then(function(res) { 
        a.push(res);
        return calculate1('FullRebuild'); 
     })
    .then(function(res) {
        a.push(res);
        document.getElementById("b").value += a.toString() + '\n'; 
    });
Community
  • 1
  • 1
JLRishe
  • 99,490
  • 19
  • 131
  • 169
  • The `Promise` constructor might actually be necessary to wrap `Excel.run`. Still, to avoid the antipattern it should look something like `resolve(ctx.sync().then(…))` – Bergi Jun 21 '16 at 23:29
  • The code works... I see `resolve(t)` is very often used to pass a value to `function (res) {...}` in `then`. Whereas, you directly use `return t`, why is it possible and how is it called? – SoftTimur Jun 21 '16 at 23:38
  • 1
    @SoftTimur The `resolve()` function is needed for creating promises out of non-promise code. Since `Excel.run()` and `ctx.sync()` are promise-based, you don't need `resolve()` here. The reason this works is that `then()` returns a promise that resolves to the value that's returned _inside_ the `then()`. A promise is returned inside the `then()`, the promise returned by `then()` will "adopt" that promise and its resolution or rejection. – JLRishe Jun 22 '16 at 03:02
  • 1
    @Bergi The [documentation](https://github.com/OfficeDev/office-js-docs/blob/master/excel/excel-add-ins-programming-overview.md) says _The run method takes in RequestContext and returns a promise (typically, just the result of ctx.sync())_. While it doesn't explicitly say that its result is the result of whatever is returned within its callback, that does appear to be the case based on SoftTimur's comment. – JLRishe Jun 22 '16 at 03:06
1

If you want an asynchronous function to return a value - instead of passing a callback to it - the return value needs to be wrapped in a promise and resolved like in your second example.

The convenience of the promise is that it avoids callback nesting -> when a callback needs another callback... And the way Errors are propagated to the closest Promise.catch() block

function calculate2 (mode) {
    return new Promise(function (resolve, reject) { 

        Excel.run(function (ctx) {       
           ctx.workbook.application.calculate(mode);
           var before = performance.now();

           ctx.sync().then(function() { 
               var after = performance.now(); 
               var t = after - before;  

               resolve(t); 
            })
        })
    })
}

calculate2('FullRebuild')
    .then(function (t) {
         document.getElementById("b").value += 'inside: ' + t + '\n';
         // the return value of a promise resolver is wrapped in a promise too
         // and the value of t will be available as parameter of the following .then(funtion(t)) block
         return t; 
 })
.catch(function(err) {
    // Addresses potential async errors raised since calling calculate2()
 });
kidroca
  • 3,480
  • 2
  • 27
  • 44