3
\$\begingroup\$

I'm new in cordova / angularJS and I wonder how much responsibility a controller can have. This particular one validates users entry data, display messaging, takes pictures calling the camera API, serialize them and finally calls a service to upload them to the cloud. I want to refactor functionality and move it to a separate service for example but not sure which functions should be moved.

  .controller('pictureOfTheVehicleCtrl', function ($scope, $cordovaCamera, $rootScope, $ionicPopup, Upload, $ionicLoading, $timeout) {

    $scope.upload = function () {
      // Validate user input
      var message = undefined;
      if ($scope.registrationNumber === undefined)
        message = 'Please, provide a valid registration number.';
      else if ($scope.damage === undefined)
        message = "Please, provide a valid picture of the damage. ";
      else if ($scope.vehicle === undefined)
        message = "Please, provide a valid vehicle picture. ";

      if (message) {
        $ionicLoading.show({ template: message });
        $timeout(function () {
          $ionicLoading.hide();
        }, 2000);
        return null;
      }

      // Upload damage picture
      var file = dataURItoBlob($scope.damage);
      $ionicLoading.show({ template: 'Uploading picture of the damage...' });
      Upload.uploadImage($rootScope.userName, $rootScope.userPass, $scope.registrationNumber, 'damage', file)
        .then(function (response) { // success
        }, function (response) {  // error
          showAlert('Error', 'Error uploading picture of the damage: ' + response)
          return null;
        }).then(function () {
          $ionicLoading.hide();
        });

      // Upload vehicle picture
      file = dataURItoBlob($scope.vehicle);
      $ionicLoading.show({ template: 'Uploading vehicle picture...' });
      Upload.uploadImage($rootScope.userName, $rootScope.userPass, $scope.registrationNumber, 'licenseplate', file)
        .then(function (response) { // success
          if (response == 0) {
            showAlert('Upload', 'Pictures successfully uploaded!')
            wipeData();
          }
          else {
            showAlert('Error', 'Error uploading picture of the vehicle: ' + response)
          }
        }, function (response) {  // error
          showAlert('Error', 'Error uploading picture of the vehicle: ' + response)
          return null;
        }).finally(function () {
          $timeout(function () {
            $ionicLoading.hide();
          }, 2000);
        });
    }

    function showAlert(title, message) {
      $ionicPopup.alert({
        title: title,
        template: message
      });
    }

    function wipeData() {
      // Wipe temp data
      $rootScope.damage = undefined;
      $rootScope.vehicle = undefined;
      $rootScope.registrationNumber = undefined;
    }

    function dataURItoBlob(dataURI) {
      // convert base64/URLEncoded data component to raw binary data held in a string
      var byteString;
      if (dataURI.split(',')[0].indexOf('base64') >= 0)
        byteString = atob(dataURI.split(',')[1]);
      else
        byteString = unescape(dataURI.split(',')[1]);

      // separate out the mime component
      var mimeString = dataURI.split(',')[0].split(':')[1].split(';')[0];

      // write the bytes of the string to a typed array
      var ia = new Uint8Array(byteString.length);
      for (var i = 0; i < byteString.length; i++) {
        ia[i] = byteString.charCodeAt(i);
      }

      return new Blob([ia], { type: mimeString });
    }  

    $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
        });
      });
    }
  })
\$\endgroup\$
2
\$\begingroup\$

You should export any code that can be used by multiple controller to a service or a utility. This will prevent code repetition, for example

Upload function

Upload.uploadImage($rootScope.userName, $rootScope.userPass, $scope.registrationNumber, 'damage', file)
  .then(function(response) { // success
  }, function(response) { // error
    showAlert('Error', 'Error uploading picture of the damage: ' + response)
    return null;
  }).then(function() {
    $ionicLoading.hide();
  });
}

You are calling this function twice and most of structure is similar. This can be made in a single function and called twice.

Note: Not the best example but can be made generic.

function uploadImage(tag, file, success) {
  Upload.uploadImage($rootScope.userName, $rootScope.userPass, $scope.registrationNumber, tag, file)
    .then(success, function(response) { // error
      showAlert('Error', 'Error uploading picture of the vehicle: ' + response)
      return null;
    }).finally(function() {
      $timeout(function() {
        $ionicLoading.hide();
      }, 2000);
    });
}

uploadImage('damage', file);
uploadImage('licenseplate', file, function(response){
   if (response == 0) {
        showAlert('Upload', 'Pictures successfully uploaded!')
        wipeData();
      } else {
        showAlert('Error', 'Error uploading picture of the vehicle: ' + response)
      }
});

Validate Function

You should export all validation code to another function and just call this function.

function ValidateInput() {
  // Validate user input
  var message = undefined;
  if ($scope.registrationNumber === undefined)
    message = 'Please, provide a valid registration number.';
  else if ($scope.damage === undefined)
    message = "Please, provide a valid picture of the damage. ";
  else if ($scope.vehicle === undefined)
    message = "Please, provide a valid vehicle picture. ";

  return message;
}

$scope.upload = function() {
  var message = validateInput();
  if (message) {
    $ionicLoading.show({
      template: message
    });
    $timeout(function() {
      $ionicLoading.hide();
    }, 2000);
    return null;
  }

Password

In this call,

Upload.uploadImage($rootScope.userName, $rootScope.userPass, $scope.registrationNumber, tag, file)

userPass should not be stored in a rootScope. Ideally, it should be saved on server and in encrypted format and using a one time random key and this encrypted value can be stored.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer, I think is very helpful. The userPass part I cannot do it as I don't have access to the backend API. I need to store user credentials when the app starts, that's why I use rootScope. If you know a better way please let me know. \$\endgroup\$ – tribet Jan 7 '16 at 16:53
  • \$\begingroup\$ you can request server for a unique generated passcode and use it. If not you can use cookies to store duration of session and validate user. But storing user credentials on client side is not only bad practice, but also dangerous. \$\endgroup\$ – Rajesh Jan 7 '16 at 16:56

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.