Two AngularJS controllers for camera actions

Question

I have the same functionality in two controllers and I want to refactor to place it in a shared method that both controllers can access. I was thinking on creating a separate service called "Camera" and inject that service as a dependecy into both controllers as I'm currently doing with "Login". Is this a good approach?

  .controller('pictureOfTheDamageCtrl', function ($scope, $cordovaCamera, $ionicPopup, $rootScope, $timeout, Login, $ionicLoading) {
    $rootScope.width = document.getElementsByTagName('ion-content')[0].clientWidth;

    $scope.takePicture = function () {
      var options = {
        quality: 75,
        destinationType: Camera.DestinationType.DATA_URL,
        sourceType: Camera.PictureSourceType.CAMERA,
        allowEdit: false,
        targetWidth: 1024,
        targetHeight: 567,
        encodingType: Camera.EncodingType.JPEG,
        popoverOptions: CameraPopoverOptions,
        saveToPhotoAlbum: false
      };

      $cordovaCamera.getPicture(options).then(function (imageURI) {
        $rootScope.damage = "data:image/jpeg;base64," + imageURI;
      }, function (err) {
        $ionicPopup.alert({
          title: 'Camera',
          template: 'Something went wrong: ' + err
        });
      });
    }

  })      
  .controller('pictureOfTheVehicleCtrl', function ($scope, $cordovaCamera, $ionicPopup, $rootScope, $timeout, Login, $ionicLoading) {
    $rootScope.width = document.getElementsByTagName('ion-content')[0].clientWidth;

    $scope.takePicture = function () {
      var options = {
        quality: 75,
        destinationType: Camera.DestinationType.DATA_URL,
        sourceType: Camera.PictureSourceType.CAMERA,
        allowEdit: false,
        targetWidth: 1024,
        targetHeight: 567,
        encodingType: Camera.EncodingType.JPEG,
        popoverOptions: CameraPopoverOptions,
        saveToPhotoAlbum: false
      };

      $cordovaCamera.getPicture(options).then(function (imageURI) {
        $rootScope.vehicle = "data:image/jpeg;base64," + imageURI;
      }, function (err) {
        $ionicPopup.alert({
          title: 'Camera',
          template: 'Something went wrong: ' + err
        });
      });
    }

  })

Answer 1

Looks ok, but here are some suggestions.

Your picture configuration looks the same for both controllers. You could move that out to an angular constant/value. If you anticipate the API mutating that config object, you can always shallow-copy them with Object.assign.

Next, proceed with your service. I'd move everything into it. All your controller has to do is call the API and receive the data URL. Move the ionic popup warning into the service as well for a centralized handling of the popup.

Another thing is the use of $rootScope. It's a quick way to share values, but like globals, $rootScope is modifiable everywhere. Your app is depending on data that may or may not be there and depends on changes whose origins you may not know. For small apps, that's fine as two-way binding is really handy but just in case your app becomes big, you might consider moving off data into a service/factory to draw a line between data and presentation.

.value('PICTURE_SETTINGS', {
  quality: 75,
  destinationType: Camera.DestinationType.DATA_URL,
  sourceType: Camera.PictureSourceType.CAMERA,
  allowEdit: false,
  targetWidth: 1024,
  targetHeight: 567,
  encodingType: Camera.EncodingType.JPEG,
  popoverOptions: CameraPopoverOptions,
  saveToPhotoAlbum: false
})
.factory('cameraFactory', function($cordovaCamera, $ionicPopup, PICTURE_SETTINGS){

  return {
    takePicture: function(){
      $cordovaCamera.getPicture(Object.assign({}, PICTURE_SETTINGS)).then(function(imageURI) {
        return "data:image/jpeg;base64," + imageURI;
      }, function(err) {
        $ionicPopup.alert({
          title: 'Camera',
          template: 'Something went wrong: ' + err
        });
      });
    }
  }
})
.controller('pictureOfTheDamageCtrl', function($scope, $rootScope, cameraFactory) {
  $rootScope.width = document.getElementsByTagName('ion-content')[0].clientWidth;

  scope.takePicture = function() {
    cameraFactory.takePicture().then(function(imageURI) {
      $rootScope.damage = imageURI;
    });
  }

})
.controller('pictureOfTheVehicleCtrl', function($scope, $rootScope, cameraFactory) {
  $rootScope.width = document.getElementsByTagName('ion-content')[0].clientWidth;

  $scope.takePicture = function() {
    cameraFactory.takePicture().then(function(imageURI) {
      $rootScope.vehicle = imageURI;
    });
  }

})