2
\$\begingroup\$

I'm creating an application with AngularJS, and packaging it up with the Phonegap build.

I'm using the facebook API (phonegap cordova-plugin-facebook4) for user authentication and getting a user's profile details.

I've tested this Facebook login process in my app on Android and on iOS and it's working perfect. However I'd like to see what you think of my code?

Here's a little bit of information on the structure:

  1. I have a factory called facebookFactory which performs all of the processing with the facebookConnectPlugin.
  2. I have a loginFactory which performs a http POST request to an endpoint of my PHP API (after the user data has been processed in the facebookFactory). On successful processing on the server side I return the user data.
  3. The user data is then returned to my mainController where it is added to scope.

A few questions:

  1. I was conscious that I didn't want to over-nest if statements in order to make the code more readable. Do you think my nesting of if statements is ok? And if not, can you suggest another way?

  2. What do you think of my use of promises in the facebookFactory? It seemed like the best way to do things (with all the callbacks from the facebook API) but if there's a better way i'd love to hear.

  3. What do you think of my use of comments? (The information feels important however I think they may be too long and would like your opinion)
  4. Do you think the code is readable and easily maintainable?
  5. Any other observations on my code, I would love to hear.

Here is the snippet of my mainController relevant to the facebook login processing:

$scope.loginWithFacebook = function(){
    /*
     * This function calls the facebookFactory processFacebookLogin() function which returns a user's faceobok public profile data 
     * It returns null if there was an error during processing.
     * We pass this data to the loginFactory checkLoginDetails function in order to make a http POST request 
     * to the server to further process the data there.
     */
    facebookFactory.processFacebookLogin().then(function(data) {
        if(data !== null){
            //If the data returned from the processFacebookLogin function is not null then continue processing the data.
            loginFactory.checkLoginDetails(data).then(function(userDetails) {
                //Store the userDetails (from the response of the http request) into our $scope.userDetails variable. 
                $scope.userDetails = userDetails;
            });
        }
    });

};

facebookFactory

/*
 * This factory consists of functions for working with the facebook API using the phonegap facebookConnectPlugin (cordova-plugin-facebook4) 
 * We take in the phonegapReady factory as a parameter which detects when phonegaps deviceready event occurs.
 * This is because we cannot use the facebookConnectPlugin before the deviceready event occurs
 * 
 */
myApp.factory('facebookFactory', function($q, phonegapReady, validatorFactory){

    //Define our private variables
    var getLoginStatus = function(){
        /* 
         * This function uses the facebookConnectPlugin.getLoginStatus function which gets a user's current facebook login status.
         * It returns a response.status of "connected" if the user is logged into our app and facebook.
         */
        var deferred = $q.defer();
        facebookConnectPlugin.getLoginStatus(
            function(response){
                //Success. Return the status of the response
                deferred.resolve(response.status); 
            }, 
            function(error){ 
                //Handle the error, outputing a message to the user if necessary
                handleFacebookError(error);
                deferred.resolve(null);      
            }
        );
        return deferred.promise;
    };


    var performFacebookLogin = function(){
        /* 
         * This function uses the facebookConnectPlugin.login function.
         * When we call the facebookConnectPlugin.login function, it firstly detects if a user is logged into facebook.
         * If they are logged in, it returns a response.status of "connected".
         * If they are not currently logged into facebook, a facebook login dialog will pop up for them to log in.
         * If they enter the correct login details in the facebook dialog, it returns a response.status of "connected".
         */
        var deferred = $q.defer();
        facebookConnectPlugin.login(["public_profile"], 
            function(response){
                //Success. Return the status of the response
                deferred.resolve(response.status);             
            },
            function(error){   
                //Handle the error, outputing a message to the user if necessary
                handleFacebookError(error);
                deferred.resolve(null); 
            }
        );
        return deferred.promise;
    };


    var getProfileDetails = function(){
        /* 
         * This function makes a call to the facebook API (using the facebookConnectPlugin.api function) 
         * and gets a user's facebook public profile data.
         * The user must be logged in first before we can call this function
         */
        var deferred = $q.defer();
        /* 
         * Note: On iOS it causes an error if there are spaces between the properties in the api call below
         * (e.g /me?fields=id, name, link, picture, ...issue 338 for cordova-plugin-facebook4 on github)
         * So we need to write it as follows: /me?fields=id,name,link,picture
         */
        facebookConnectPlugin.api('/me?fields=id,name,link,picture',["public_profile"],
            function(data){
                //Success. Validate and prepare the data before returning it
                var preparedData = validateFacebookDetails(data);
                deferred.resolve(preparedData); 
            },
            function(error){
                //Handle the error, outputing a message to the user if necessary
                handleFacebookError(error);
                deferred.resolve(null); 
            }
        ); 
        return deferred.promise;
    };



    var handleFacebookError = function(error){
        /* 
         * The facebook API returns an error as an object on android and as a string on iOS.
         * So this function firstly checks what "type" the error is (object or string) and then handles it
         * Also looking out for the "User cancelled dialog" error (errorCode: 4201 on android),
         * as we do not want to output a message to the user when this error occurs.
         */
        if(error !== null && typeof error === 'object'){
            //Error is an object so therefore we are on android 
            if(!(error.hasOwnProperty('errorCode') && error.errorCode === "4201")){
                //This is not a "User cancelled dialog" (4201) error so we will output the error to the user   
                alert("We're sorry but the following error occured when trying to process your request: " + error.errorMessage); 
            }
        }else{
            //Error is not an object therefore we are on iOS and the error is a string.
            //Check if the error string equals "User cancelled" because this is what iOS returns when user cancels out of logging in to facebook. 
            if(error.indexOf("User cancelled") == -1){
                //The error string does not contain "User cancelled" so we will output an error message.
                alert("We're sorry but the following error occured when trying to process your request: " + error);
            }
         }
    };


    var validateFacebookDetails = function(userDetails){
        /* 
         * This function checks if the facebook user details are valid
         * If they are valid then we prepare them (for sending to the server later, in our http request in the loginFactory)
         * and return an object called data.
         * If they are not valid we return null.
         */
        var facebookUserID = userDetails.id;
        var facebookName = userDetails.name;
        //The "picture" returned from the facebook api call is the tiny thumbnail so instead of using that we get a larger size picture
        //by using the user's id in the following way:
        var profilePicURL = "https://graph.facebook.com/" + userDetails.id + "/picture?type=large&w‌​idth=200&height=200";  


        var inputsAreValid = validatorFactory.validateFacebookInputs(
            [{"input" : facebookUserID, "minLength" : 1, "maxLength" : 30, "regex" : /^\d+$/},
            {"input" : facebookName, "minLength" : 1, "maxLength" : 60},
            {"input" : profilePicURL, "minLength" : 1, "maxLength" : 250}]);
        if(inputsAreValid){
            //After inputs are checked for validity then we prepare the data
            var data = {
                "facebookUserID" : facebookUserID, 
                "facebookName" : facebookName, 
                "profilePicURL" : profilePicURL
            };
            return data;
        }else{
            //If the inputs are not valid then return null
            return null;
        }
    };



    var processFacebookLogin = phonegapReady(function(){
        /* 
         * This function checks a user's facebook login status, logs them into facebook if necessary, retrieves and returns their public profile data.
         * If an error occurs along the way, we return null so that we can check for this in our mainCntroller (when this function is called).
         * We wrap this function in the phonegapReady function so that it doesn't get called before the phonegap deviceready event occurs.
         */
        var deferred = $q.defer();

        getLoginStatus().then(function(responseStatus){   
            //Check the current login status of the user
            if(responseStatus === "connected"){
                //the user is logged into facebook and our app

                getProfileDetails().then(function(preparedData){ 
                    //preparedData will be null if there was an error when getProfileDetails() was called
                    //or if the user data (returned from the api call) was invalid after calling validateFacebookDetails
                    deferred.resolve(preparedData); 
                });
            }else if(responseStatus !== null){
                //If the user is not "connected" then log the user into facebook and our app
                performFacebookLogin().then(function(responseStatus){
                    if(responseStatus === "connected"){
                        //the user is now logged into facebook and our app
                        getProfileDetails().then(function(preparedData){
                            //preparedData will be null if there was an error when getProfileDetails() was called
                            //or if the user data (returned from the api call) was invalid after calling validateFacebookDetails
                            deferred.resolve(preparedData); 
                        });

                    }else{
                        //responseStatus is not "connected" so we cannot retrieve the profile data. Therefore return null.
                        //We will check for null when this function is called in the controller and stop the rest of the processing there.
                        deferred.resolve(null);                   
                    }
                });
            }else{
                //responseStatus is null so an error occured. Therefore return null.
                //We will check for null when this function is called in the controller and stop the rest of the processing there.
                deferred.resolve(null);   
            }
        });

        return deferred.promise;
    });


    //return public API so that we can access it in all controllers
    return{
        processFacebookLogin: processFacebookLogin
    };
});

loginFactory

/*
 * This factory consists of functions which will process a users login details or the userToken stored in local storage
 */
myApp.factory('loginFactory', function($http, sharedFactory, userFactory){

    var checkLoginDetails = function(data){
        /*
         * This function makes a POSt request to the facebookAuth endpoint to process a user's facebook login details
         * If the facebook details are validated on the server side we get return the profile data for that user
         * We then populate the userService.userDetails object with this data 
         * If an error occurs in the process we return null (instead of the userDetails)
         */
        return $http({
                method: 'POST',
                url: 'http://localhost/API/facebookAuth?apiKey=66reu6re',
                data: JSON.stringify(data),
                headers: {
                    'Content-Type': 'application/json;charset=utf-8'
                },
                responseType:'json'
            }).then(function successCallback(response){
                //Do checks to see if the response data is valid. If not valid then build the error notification
                var responseUserDataIsValid = false; //initialize boolean to false
                if(response.hasOwnProperty('data') && response.data !== null){
                    //check if the data property is present in the response and it is not null
                        if(response.data.hasOwnProperty('userID') && response.data.userID !== null){
                           //Check if the userID property exists and also is not null in the response.
                            //This is the best way to determine whether the user details were successfully generated. 
                            responseUserDataIsValid = true;
                        }
                }else{
                    //data property is not present in the response or it is present but it is null
                    sharedFactory.buildErrorNotification(response);
                }
                //After the checks to see if the response is valid then store our user data in the userDetails object of the userFactory.
                if(responseUserDataIsValid){               
                    userFactory.userService.setUserDetails(response.data);
                    //store the userToken in local storage.
                    setUserToken(userFactory.userService.userDetails.userToken);
                    return userFactory.userService.userDetails;

                }else{
                    //The data is not valid (or there has been an error)
                    sharedFactory.buildErrorNotification(response);
                    //The controller is expecting the userDetails returned. We pass in null instead.
                    return null;
                }

            },function errorCallback(response){
                sharedFactory.buildErrorNotification(response);
                //The controller is expecting the userDetails returned. We pass in null instead.
                return null;
            });
    };


    var setUserToken = function(userToken){
        //setting the userToken value in localStorage like this will overwrite any value thats currently there (if the "userToken" key exists)
        localStorage.setItem('userToken', userToken);
    };


    //return public API so that we can access it in all controllers
    return{
        checkLoginDetails: checkLoginDetails
    };
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.