3
\$\begingroup\$

I recently put in a submission for a small coding challenge using Angular. The challenge was to get a token and array of values using a GET request then make another GET request passing the token and sum of values within the url. I'm still getting to grips with promises and want to know if the flow in the service and controller code below can be improved. For me the problematic line is returning the helper function solveChallenge from within the getChallenge function. It works but is my use of promises good or an anti-pattern? Should I be using the success and error methods provided by $http in place of then and catch? Any other feedback on the code is definitely much appreciated.

dataservice.js

(function() {
    'use strict';

    angular
      .module('app')
      .factory('dataservice', dataservice);

    dataservice.$inject = ['$http', '$log'];

    function dataservice($http, $log) {  
        var sum, token, challenge;

        return  {
            getChallenge: getChallenge
        };

        function getChallenge(url) {

            return $http.get(url)
                    .then(function(data, status, headers, config) {
                        token = data.data.token;
                        sum = sumArray(data.data.values);
                        challenge = {token, sum};
                        return solveChallenge(url, challenge);
                    })
                    .catch(function(data, status, headers, config) {
                        $log.error("The request failed. Error code: " + status);
                    });
        }

        function solveChallenge(url, challenge) {
            var sum, token, solutionUrl;

            sum = challenge.sum;
            token = challenge.token;
            solutionUrl = url + '/' + token + '/' + sum;

            return $http.get(solutionUrl)
                    .then(function(data, status, headers, config) {
                        return data.data.answer;
                    })
                    .catch(function(data, status, headers, config) {
                        $log.error("The request failed. Error code: " + status);
                    });

          }
    }

    function sumArray(items) {
        var sum = 0;
        for (var i = 0; i < items.length; i++) {
            sum += items[i];
        }
        return sum;
    }

})();

controller.js

(function() {
    'use strict';

    angular.module('app').controller('Controller', Controller);

    Controller.$inject = ['dataservice'];

    function Controller(dataservice) {
        var vm = this;

        vm.showMeTheChallenge = function() {

          dataservice.getChallenge('http://the-url/challenge')
            .then(function(response) {

              vm.challengeSolution = response;

          });

        };
        vm.resetChallenge = function() {

            vm.challengeSolution = null;

        }

    }

})();
\$\endgroup\$
3
\$\begingroup\$

It works but is my use of promises good or an anti-pattern?

Promises work great when using $http.

Should I be using the success and error methods provided by $http in place of then and catch?

According to the documentation:

Returns a promise object with the standard then method and two http specific methods: success and error. The then method takes two arguments a success and an error callback which will be called with a response object. The success and error methods take a single argument - a function that will be called when the request succeeds or fails respectively. The arguments passed into these functions are destructured representation of the response object passed into the then method.

So you should use .then(successHandler, errorHandler) or .success(successHandler) and .error(errorHandler), but .catch(errorHandler) will not work with $http! According to the docs anyway...

function solveChallenge(url, challenge) is confusing to me, is this function really solving the challenge or is it checking the solution against a server? Consider renaming to checkSolution(url, challenge)

function getChallenge(url) is also confusing, it gets the challenge, solves it, and returns an answer from a server to see if the solution was correct. Should it be renamed to getAndSolveChallenge(url)?

function sumArray(items) could be rewritten as:

function sumArray(items) {
  return items.reduce(function(previousValue, currentItem) {
     return previousValue + currentItem;
  }, 0);
}

but there is nothing wrong with your sumArray function as is.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks this is great. Good catch on catch :). Yes my sumArray method was naive. A reduce method is a much better approach. \$\endgroup\$ – br3w5 Jul 17 '15 at 15:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.