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;
 });
 }
})

• tribet: Thanks for your answer but I cannot make it work. If I include the new PICTURE_SETTINGS value into my services.js file, then I got an "Uncaught ReferenceError: Camera is not defined". I've included the ngCordova dependency within the module signature but still same error. I've tried without the PICTURE_SETTINGS value and replacing the "Camera" properties by its numeric values and it works but the image is not displayed.