0

I've been told that this function (or one very similar) is an example of deferred anti-pattern

function writeFile(file) 
{
    return new WinJS.Promise(function (complete, error, progress) {
        if (file) {
            Windows.Storage.FileIO.writeTextAsync(file, getEditorContent()).done(
                function () {
                    currentFile = file;
                    WinJS.log && WinJS.log("writeFile: file written: " + file.name + "", "MyApp", "info");
                    complete();
                },                      
                function (error) {
                    WinJS.log && WinJS.log("writeFile: error writing File " + file.name + "", "MyApp", "error");
                    error();
                }
            );
        }
        else {
            WinJS.log && WinJS.log("writeFile: error writing File: file object was null", "MyApp", "error");
            error();
        }
    });
}

I need all three branches of this to return a promise, as this method is part of a promise chain. If I want to avoid the dreaded anti-pattern, I should supposedly do this:

function writeFile(file) 
{
    if (file) {
        return Windows.Storage.FileIO.writeTextAsync(file, getEditorContent());
    }
    else {
        WinJS.log && WinJS.log("writeFile: error writing File: file object was null", "MyApp", "error");
        return WinJS.Promise.as();
    }
}

And move my done() code to the calling function. However, writeFile is called from a number of locations in my app, and so in the interests of DRY performs some functions after writeTextAsync. I don't want to move these to the calling function, so I'm left doing this.

function writeFile(file) 
{
    if (file) {
        Windows.Storage.FileIO.writeTextAsync(file, getEditorContent()).done(
            function () {
                currentFile = file;
                WinJS.log && WinJS.log("writeFile: file written: " + file.name + "", "MyApp", "info");
                return WinJS.Promise.as();
            },                      
            function (error) {
                WinJS.log && WinJS.log("writeFile: error writing File " + file.name + "", "MyApp", "error");
                return WinJS.Promise.as();
            }
        );
    }
    else {
        WinJS.log && WinJS.log("writeFile: error writing File: file object was null", "MyApp", "error");
        return WinJS.Promise.as();
    }
}

It seems like I've just swapped a wrapped promise for three returned ones. How or why is this better?

UPDATE

I think I just realised I can return a then promise that writeTextAsync returns:

function writeFile(file) 
{
    if (file) {
        // I can return this, right?
        return Windows.Storage.FileIO.writeTextAsync(file, getEditorContent()).then(
            function () {
                currentFile = file;
                WinJS.log && WinJS.log("writeFile: file written: " + file.name + "", "MyApp", "info");
            },                      
            function (error) {
                WinJS.log && WinJS.log("writeFile: error writing File " + file.name + "", "MyApp", "error");            
                return WinJS.Promise.wrapError(error)
            }
        );
    }
    else {
        WinJS.log && WinJS.log("writeFile: error writing File: file object was null", "MyApp", "error");
        return WinJS.Promise.wrapError("error writing File: file object was null")
    }
}
11
  • 1
    Did you see the question about the explicit construction anti pattern in stackoverflow before? Sep 14, 2015 at 17:02
  • Possible duplicate of How do I avoid the Promise constructor antipattern?
    – Bergi
    Sep 14, 2015 at 21:06
  • @BenjaminGruenbaum I presume you mean this one - stackoverflow.com/questions/23803743/… - but in my case I don't wish to repeat my done logic, so I want it to remain in writeFile. I also want to log an error if no file object is passed. I'll try and put a simpler example into my question
    – roryok
    Sep 15, 2015 at 8:34
  • @BenjaminGruenbaum actually scratch that, I think I just figured it out. see updated answer.
    – roryok
    Sep 15, 2015 at 8:48
  • @roryok:Looks good - except that you want to use Promise.wrapError() (the WinJs equivalent of ES6 Promise.reject()) instead of Promise.as(), and that you'll need to re-throw error in your error handler so that the exception propagates like in your original code.
    – Bergi
    Sep 15, 2015 at 15:25

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.