advanced-analytics-toolbox icon indicating copy to clipboard operation
advanced-analytics-toolbox copied to clipboard

Don't use $q if not needed

Open nilzona opened this issue 6 years ago • 0 comments

Hello, I work at R&D at Qlik and I'm scanning through Qlik Branch to look at some of our live extensions out there ...

I noticed that you're using the $q module without actually needing it. All our backend API calls in the extension API are already $q.defer promises so you can use them instead of declaring it as a dependency. "ng!$q" is nowadays an unsupported and deprecated way of using this plugin so I recommend to remove it.

For example in lib/js/analysis/autocorrelation.js:

/**
     * drawChart - draw chart with updated data
     *
     * @param {Object} $scope angular $scope
     *
     * @return {Object} Promise object
     */
    drawChart($scope, app) {
      const defer = $q.defer();
      const layout = $scope.layout;

      const dimension = utils.validateDimension(layout.props.dimensions[0]);
      const requestPage = [{
        qTop: 0,
        qLeft: 0,
        qWidth: 6,
        qHeight: 1500,
      }];
      
      // This call will return a $q.defer promise so return this instead of returning promise further down
      return $scope.backendApi.getData(requestPage).then((dataPages) => {
       
       // code goes here

        
        return defer.resolve();   // <----- this is not needed, the promise will resolve with whatever you return or just resolve with undefined value if you don't return anything here.
      }); // end of backend API callback
      return defer.promise; // not needed, return the function call as shown above instead.
    }, // end of drawChart function

with these changes you can remove the "ng!$q" module entirely.

nilzona avatar Jun 19 '18 21:06 nilzona