Angular - Best practice for retrieving data from a Factory method

It depends on what your controller is expecting and how you set up your application. Generally, I always go with the second option. Its because I usually have global error or success handlers in all api requests and I have a shared api service. Something like below.

var app = angular.module('app', []);app.service('ApiService', ['$http', function($http) {    var get = function(url, params) {    $http.get(url, { params: params })        .then(handleSuccess, handleError);  };  // handle your global errors here  // implementation will vary based upon how you handle error  var handleError = function(response) {    return $q.reject(response);  };  // handle your success here  // you can return response.data or response based upon what you want  var handleSuccess = function(response) {    return response.data;  };}]);app.service('InfoService', ['ApiService', function(ApiService) {    var retrieveInfo = function() {    return ApiService.get(retrievalFile);    /**    // or return custom object that your controller is expecting    return ApiService.get.then(function(data) {      return new Person(data);    });    **//  };  // I prefer returning public functions this way  // as I can just scroll down to the bottom of service   // to see all public functions at one place rather than  // to scroll through the large file  return { retrieveInfo: retrieveInfo };}]);app.controller('InfoController', ['InfoService', function(InfoService) {  InfoService.retrieveInfo().then(function(info) {    $scope.info = info;  });}])

Or if you are using router you can resolve the data into the controller. Both ngRouter and uiRouter support resolves:

$stateProvider.state({    name: 'info',  url: '/info',  controller: 'InfoController',  template: 'some template',  resolve: {    // this injects a variable called info in your controller    // with a resolved promise that you return here    info: ['InfoService', function(InfoService) {        return InfoService.retrieveInfo();    }]  }});// and your controller will be like// much cleaner rightapp.controller('InfoController', ['info', function(info) {    $scope.info = info;}]);

It's really just preference. I like to think of it in terms of API. What is the API you want to expose? Do you want your controller to receive the entire response or do you want your controller to just have the data the response wraps? If you're only ever going to use response.data then option 2 works great as you never have to deal with anything but the data you're interested in.

A good example is the app we just wrote where I work. We have two apps: a back-end API and our front-end Angular application. We created an API wrapper service in the front-end application. In the service itself we place a .catch for any of the API endpoints that have documented error codes (we used Swagger to document and define our API). In that .catch we handle those error codes and return a proper error. When our controllers/directives consume the service they get back a much stricter set of data. If an error occurs then the UI is usually safe to just display the error message sent from the wrapper service and won't have to worry about looking at error codes.

Likewise for successful responses we do much of what you're doing in option 2. In many cases we refine the data down to what is minimally useful in the actual app. In this way we keep a lot of the data churning and formatting in the service and the rest of the app has a lot less to do. For instance, if we need to create an object based on that data we'll just do that in return the object to the promise chain so that controllers aren't doing that all over the place.

I would choose option two, as it your options are really mostly the same. But let see when we add a model structure like a Person suppose.

comparison.factory('Info', ['$http', function($http) {var retrievalFile = 'retrievalFile.json';return {  retrieveInfo: function() {    return $http.get(retrievalFile).then(function(response) {      //we will return a Person...      var data = response.data;      return new Person(data.name, data.age, data.gender);    });  }}}]);