Use Promise or callbacks in this use-case?

Question

I have this use-case where I am not sure if it's better to use a Promise or not. The problem I see, is that I have a 3-case scenario:

success
failure
abort(nothing happens)

My code-sample is here:


class MyView{

// ...

    /**
     * Saves the record changes. Process the uploads. 
     * If beforeSave returns true, nothing should happen.
     * If the view has uploads, process them before saving the record.
     *
     * @param {Callback|Function} success
     * @param {Callback|Function} failure
     */
    saveRecord: function (success, failure) {
        const me = this,
            view = me.getView();

        if (false === view.fireEvent('beforeSave')) {
            //this is the abort scenario
            return;
        }

        if (view.hasUploads()) {
            view.processUploads(() => {
                me.executeRecordSave(success, failure);
            });
        } else {
            me.executeRecordSave(success, failure);
        }
    },

    /**
     * Save the Record Effectively
     * @private
     * @param {Callback|Function} success
     * @param {Callback|Function} failure
     */
    executeRecordSave: function (success, failure) {
        const me = this,
            view = me.getView();

        const record = me.vmGet('record');

        RecordSaver.saveRecord(record, view, {
            success: success,
            failure: failure
        });
    },

// ...

}

The current usage is:

const view = new MyView();

view.saveRecord(() => {
    console.log('success');
}, () => {
    console.log('success');
});

and I would like to replace it with a Promise:

const view = new MyView();

// would like to replace it with a Promise:
view.saveRecord().then(() => {
    console.log('success');
}, () => {
    console.log('success');
});

but in case of "abort" this Promise will never be fulfilled.

The only problem I see with the promise is this piece of code which will never fulfill the promise.

        if (false === view.fireEvent('beforeSave')) {
            return;
        }

so I am not sure if I actually use a Promise in this case.

Or, other way, I could return a Promise which will NEVER be fulfilled in case of "abort". Is that ok?

user_12594730 · Answer

I'm assuming both view.processUploads and RecordSaver.saveRecord return a promise (technically they should)

class MyView{

    // ...

    /**
     * Saves the record changes. Process the uploads. 
     * If beforeSave returns true, nothing should happen.
     * If the view has uploads, process them before saving the record.
     *
     * @param {Callback|Function} success
     * @param {Callback|Function} failure
     */
    saveRecord: async () => {
        // use arrow function so you don't need the "me = this"
        const view = this.getView();

        if (false === view.fireEvent('beforeSave')) {
            //this is the abort scenario
            return;
        }

        if (view.hasUploads())
            // Wait for the upload to end
            // the result of the upload should be 
            // saved in a private variable
            // to be used by _executeRecordSave()
            await view.processUploads();
        
        return this._executeRecordSave();
        
    },

    /**
     * Save the Record Effectively
     * @private
     */
    _executeRecordSave: () => {
        const view = this.getView();

        const record = this.vmGet('record');
        // return the saveRecord result as a Promise
        return RecordSaver.saveRecord(record, view);
    },

    // ...

}

Usage :

const view = new MyView();

view.saveRecord().then(result => {
    if(result === 'success') () => console.log('success') // success function
    if(result === 'failure') () => console.log('failure') // failure function
    if(!result) () => console.log('abort') // abort function
});

Also you need to add proper error handler for the upload errors.

Ps: Can't test this code, it should work, sorry in advance if there is any error.

Use Promise or callbacks in this use-case?

1 Answers