skip to Main Content

I’m just inherited a new codebase and this pattern in javascript seems to do nothing:

/**
 * Update Data Model
 * @param {Object} model
 * @param {String} field
 * @param {Mixed} value
 */
function updateDataModel(model, field, value) {
  return new Promise((resolve) => {
    async function promiseFunc() {
      model[field] = value;
      await model.save();
      resolve(model);
    }

    return promiseFunc();
  });
}

module.exports = updateDataModel;

It’s everywhere in the code base and I’m trying to be charitable, but I can’t imagine any use of wrapping the async await within the promise? It’s confusing and doesn’t seem to do anything.

In my mind the above code pattern is basically equivalent to either:

/**
 * Update Data Model
 * @param {Object} model
 * @param {String} field
 * @param {Mixed} value
 */
async function updateDataModel(model, field, value) {
      model[field] = value;
      await model.save();
      return model
}

module.exports = updateDataModel;

—or—

/**
 * Update Data Model
 * @param {Object} model
 * @param {String} field
 * @param {Mixed} value
 */
function updateDataModel(model, field, value) {
      model[field] = value;
      const newModel = model.save().then((model)=> return model);
      return newModel;
}

module.exports = updateDataModel;

Am I missing something important here? Everything is very buggy and moving to a simpler pattern is one of the first steps the team is taking to try and improve. Error checking i somewhat difficult because the error is an unhandled promise in most cases.

2

Answers


  1. If the save promise returns the updated model, I would just return it.

    /**
     * Update Data Model
     * @param {Object} model
     * @param {String} field
     * @param {Mixed} value
     */
    function updateDataModel(model, field, value) {
      model[field] = value;
      return model.save(); // Return the promise here (returns the underlying model)
    }
    
    const animal = {
      type: 'cat',
      color: 'white',
      save() {
        // Do some DB updates...
        return Promise.resolve(this);
      },
      toString() {
        return `Model(type=${this.type},color=${this.color})`;
      }
    };
    
    updateDataModel(animal, 'color', 'black').then(model => {
      console.log('Updated model:', model.toString());
    });

    Or:

    const model = await updateDataModel(animal, 'color', 'black');
    console.log('Updated model:', model.toString());
    
    Login or Signup to reply.
  2. Since async/await is syntactic sugar around a Promise, the existing code is redundant and can be rewritten like you suggest.

    Each time when an async function is called, it returns a new Promise which will be resolved with the value returned by the async function, or rejected with an exception uncaught within the async function.

    Source: mdn

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search