Pulling data from different JSON files

Question

I have a controller that pulls gets data from two different JSON files. One of the calls loadProducts happens when the controller is initiated. The other one, loadPrices, is called when a user clicks a button on the interface.

I have noticed that the controller is starting to get bigger and I was worried about the state of the code. As I am knew to this I was hoping for suggestions about how I can clean it up or make it more efficient.

'use strict';
(function () {

var app = angular.module('priceQuoteApp');

var quoteBuilderController = function ($scope, $http, $timeout) {

    //set the variabls
    $scope.listOfProducts = null;
    $scope.selectedProduct = null;
    $scope.listOfProductVariants = null;
    $scope.formData = {};
    $scope.showTable = 'none';
    $scope.selectedProductId = null;
    $scope.loadingProducts = true;
    $scope.loadedProducts = false;
    $scope.loadingPrices = false;
    $scope.loadedPrices = false;

    // get all the portfolios and the products
    // the timer is to simulate network latency
    var loadProducts = function () {
      $http.get('scripts/json/sample-products.json')
        //$http.get('http://www.thesite/api/Pricing/GetProductCatalogue')
            .then(function (allProducts) {
                $scope.listOfProducts = allProducts.data.Connectivity;
            })
            .finally(function () {
                $scope.loadingProducts = false;
                $scope.loadedProducts = true;
            });
    };
    //loadProducts();
   $timeout(loadProducts, 2000);

    // get all the prices
    // the timer is to simulate network latency
    var loadPrices = function () {
        $http.get('scripts/json/sample-product-prices.json')
            .then(function (res) {
                $scope.selectedProductPrices = res.data.ProductVariants;
                $scope.selectedProductAddOns = res.data.product_addons;
            })
            .finally(function () {
                $scope.loadingPrices = false;
                $scope.loadedPrices = true;
            });
    };

    // get the product details
    $scope.getProductDetails = function (item) {
        $scope.selectedProduct = item.product_name;
        $scope.selectedProductId = item.product_id;
        $scope.listOfProductVariants = item.product_variants;
        $scope.formFields = item.product_price_attributes;
    };

    // select the product and get the price
    $scope.selectProduct = function (item) {
        $scope.selectedRow = item;
        $scope.selectedProductPrice = item.RecurringCost;
    };

    // replace this with api calls to get the prices
    $scope.getProductVariants = function () {
        $scope.loadingPrices = true;
        $timeout(loadPrices, 2000);
    };
};

app.controller('quoteBuilderController', ['$scope', '$http', '$timeout', quoteBuilderController]);

}());

One this for sure is that I should probably break it up as its becoming long. Is there a need to define the variables at the top? Should I be using the This keyword?

Use services/factories to encapsulate your $http service calls. Also, there is no need to break up your object model when assigning to a property on $scope. And try to add more than one ngController, that one controller is doing too much. Instead of doing things like "loading=true" - which I assume is for displaying a wait dialog or spinner, consider implementing an $http loading directive, outside of your very large controller — pixelbits
– pixelbits, Commented Feb 3, 2015 at 8:31
Sorry Jurge - I didnt know that existed. Pixelbits - many thanks for your detailed answer. I will implement some of that. — Jose the hose
– Jose the hose, Commented Feb 3, 2015 at 8:44
Seems you are right 200. I forgot about that. Thanks for looking into and clarifying that for us. — Jose the hose
– Jose the hose, Commented Feb 3, 2015 at 10:07
Hey Pixelbits - what do you mean by 'there is no need to break up your object model when assigning to a property on $scope'? thank you — Jose the hose
– Jose the hose, Commented Feb 4, 2015 at 15:38

Cizaphil · Accepted Answer · 2015-02-03 08:48:23Z

There is no need using the 'this' keyword, or such as angularjs provides for those scenario. One thing you can do is to abstract the loadProducts functions and load prices function to a factory and then pass the factory as a parameter to your controller as shown below.

var priceQuoteApp = angular.module('priceQuoteApp', [])
    .constant("loadProductUrl", "scripts/json/sample-products.json")
    .constant("loadPricesUrl", 'scripts/json/sample-product-prices.json');

priceQuoteApp.factory("ProductsFactory", function ($http, loadProductUrl, loadPricesUrl) {
        return {
            loadProducts: function () {
                //Create an object to hold your result
                var result = {};

                $http.get(loadProductUrl)  
                .then(function (allProducts) {
                    result.listOfProducts = allProducts.data.Connectivity;
                })
                    .finally(function () {
                        result.loadingProducts = false;
                        result.loadedProducts = true;
                    });
            },

            loadPrices: function () {
                $http.get(loadPricesUrl)
                    .then(function (res) {
                        result.selectedProductPrices = res.data.ProductVariants;
                      result.selectedProductAddOns = res.data.product_addons;
                    })
                    .finally(function () {
                        result.loadingPrices = false;
                        result.loadedPrices = true;
                    });
            }
        });

Then you inject the factory into your controller as shown below

priceQuoteApp.controller("quoteBuilderController", function ($scope, $http, $timeout, ProductsFactory) {
        var products = ProductsFactory.loadProducts();
        //Your other codes

        //When the user clicks the button
        $scope.clickAction = function () {
            var prices = ProductsFactory.loadPrices;
            //Then do other things with it.
        }
    });

One good thing is that this increases code return and thereby making every part of your code to focus on a single responsibility. Hope this helps.

Anil Singh · Accepted Answer · 2015-02-04 04:46:20Z

It might help you!!! Here I'm using services and controller in separate parts.

var app = angular.module('priceQuoteApp');

app.service('quoteBuilderServices', function ($http) {
    this.getProducts = function () {
        return $http.get('scripts/json/sample-products.json').
        success(function(resp){
            //log something here.
        });
    };
});

app.controller("quoteBuilderController", function($scope, $timeout, quoteBuilderServices) {

   //Set the variabls
    $scope.listOfProducts = null;
    $scope.loadingProducts = true;
    $scope.loadedProducts = false;

   // Calling here services method.
    quoteBuilderServices.getProducts()
      .then(function (allProducts) {
         $scope.listOfProducts = allProducts.data.Connectivity;
      })
      .finally(function () {
         $scope.loadingProducts = false;
         $scope.loadedProducts = true;
      });       
});

Stack Exchange Network

Pulling data from different JSON files

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Pulling data from different JSON files

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions