0

I am a new developer and am working on a rather complex scenario where, when the user saves, there might be a lock and the user has the chance to over ride the lock. If there is a lock, since REST is stateless, the object I was sending on the PUT is lost, so I have to allow the user to over ride the lock and then make the put request again.

In the second if check you can see that I have a nested promise. From what I know about promises vs. callbacks, this defeats the purpose of using promises. I read through some other answers, but did not understand the concept of returning a promise in the inner/nested promise. How could I refactor the code below to make it more in line with best practices and not nest promises?

//the user chooses to over ride someone else's lock
  $scope.$on('forceLockAfterModalSubmit', function (e, data) {
    if (!$scope.newItemCreatedIsLocked) {
      $scope.setLockForCurrentUser();
      $scope.editMode = true;
    }
    if ($scope.newItemCreatedIsLocked) {
      service.forceLock($scope.id).then(function () {
        itemService.updateItem($scope.itemForRequestBeforeLockResponse).then(function () {
          $scope.postPUTRequestActions($scope.itemForRequestBeforeLockResponse);
        })
      }, function (err) {
        console.log(err);
      })
    }
  })
devdropper87
  • 4,025
  • 11
  • 44
  • 70
  • 2
    possible duplicate of [Removing nested promises](http://stackoverflow.com/a/22000931/1048572) – Bergi May 16 '16 at 20:25

2 Answers2

2

You are mixing callbacks and promises and making it harder than it has to be. All of your asynchronous functions should return a promise, and instead of using the second .then() as an error handler you should let a .catch() function handle the errors.

The code you have at the moment could be replaced by

$scope.$on('forceLockAfterModalSubmit', function(e, data) {
  if (!$scope.newItemCreatedIsLocked) {
    $scope.setLockForCurrentUser();
    $scope.editMode = true;
  }
  if ($scope.newItemCreatedIsLocked) {
    service.forceLock($scope.id)
      .then(function() {
        return itemService.updateItem($scope.itemForRequestBeforeLockResponse);
      })
      .then(function() {
        return $scope.postPUTRequestActions($scope.itemForRequestBeforeLockResponse);
      })
      .catch(function(err) {
        console.log(err);
      });
  }
});

If you want a more clean solution, you could declare a function that calls your itemService.updateItem and $scope.postPUTRequestActions with the scoped id and you would end up with

$scope.$on('forceLockAfterModalSubmit', function(e, data) {
  if (!$scope.newItemCreatedIsLocked) {
    $scope.setLockForCurrentUser();
    $scope.editMode = true;
  }
  if ($scope.newItemCreatedIsLocked) {
    service.forceLock($scope.id)
      .then(itemService.updateItem)
      .then($scope.postPUTRequestActions)
      .catch(function(err) {
        console.log(err);
      });
  }
});

which is both easy to understand and to follow.

Daniel B
  • 8,770
  • 5
  • 43
  • 76
  • "*you should handle any error in a `.catch()`*" - not necessarily, there are very good reasons to use the second `then` argument as you sometimes need [something different](http://stackoverflow.com/q/24662289/1048572). But you're right, a generic error handler should always go in a `catch` at the very end of the chain. – Bergi May 16 '16 at 20:26
  • @Bergi, good point and example, though in this case the second argument is only used as an error handler which I wanted to address. I updated my answer! – Daniel B May 16 '16 at 21:11
  • thanks. unfortunately I'm using jquery promises in my project so catch won't work :) – devdropper87 May 16 '16 at 23:22
  • 2
    @devdropper87 _"I'm using jquery promises in my project so catch won't work"_ jQuery 3x has `.catch()` defined as method at `$.Deferred()` – guest271314 May 17 '16 at 07:05
-1

Pretty new to this as well but maybe this might be useful.

Another idea is that in the first function that returns a promise you might want to call the other function and use setTimeout(function2, "insert milliseconds here") though that can slow things down in the long run since you want it as soon as the data is ready... Seems hacky to me but it might be a bandage for the short term.

On a somewhat related note you might want to write things out like this to help with readability.

service.then(successFunction).catch(errorFunction);

function successFunction(response) {
    if (response == "Valid Data") {
      return response;
    } else {
      console.log("Something is wrong with the success function!");
      return response;
    }

    functon errorFunction(response) {
      console.log("Error occurred in the service!");
    }

Hope this helps some what.

Jeff L
  • 141
  • 2
  • 12