Skip to main content
added 948 characters in body
Source Link

Couple of more points -

  1. Why have you given time delay of 750 to your timeout function. It might cause multiple duplicate items (same items might show up as different individual row instead of incremented count within one row) on UI if user keeps clicking on "add item" (which triggers addToCart scope function) frequently.
  2. Why are you keeping reference of localStorage at scope level? Are you using it anywhere in scope ? (referring to this - $scope.localStorage = $localStorage)
  3. You can improve time complexities of following scope methods (itemWithoutBtw, total, totalBtw etc) and avoid O(n) iteration if you modify your add / remove code diligently. If you still want to iterate on entire cart from code maintainability aspect, you can introduce one variable to read cart state, and cache these values to avoid unnecessary iteration on entire cart until cart state changes.
  4. Supporting @konijn point - if you want to store price in localStorage, ensure that you have server side logic to override price and other sensitive cart item details to avoid hacks there. Try showing up overridden price to user as well ( by syncning with server ) when you user is about to pay. That ways you will lead to a better user experience and less dissatisfaction among users.

$scope.itemWithoutBtw = 0;
$scope.addToCart = function(index, title, desc, price, timeout) {
  // Same code as of above, removing this to save space here, Below I wrote for single quantity case. You can write similar logic if item already exists.
  if (!found) {
    $timeout(function() {
        $localStorage.items.push(angular.extend({
        id: index,
        title: title,
        quantity: 1,
        price: price}, index))
        // Below line is added here additionally..
        $scope.itemWithoutBtw += price / 106 * 100;
        $scope.checked = false;
        },750);
    }
};

// You dont need this function and can use scope var directly.
$scope.itemWithoutBtw = function(index) {
    return $scope.itemWithoutBtw;
};

  1. Supporting @konijn point - if you want to store price in localStorage, ensure that you have server side logic to override price and other sensitive cart item details to avoid hacks there. Try showing up overridden price to user as well ( by syncning with server ) when you user is about to pay. That ways you will lead to a better user experience and less dissatisfaction among users.

Couple of more points -

  1. Why have you given time delay of 750 to your timeout function. It might cause multiple duplicate items (same items might show up as different individual row instead of incremented count within one row) on UI if user keeps clicking on "add item" (which triggers addToCart scope function) frequently.
  2. Why are you keeping reference of localStorage at scope level? Are you using it anywhere in scope ? (referring to this - $scope.localStorage = $localStorage)
  3. You can improve time complexities of following scope methods (itemWithoutBtw, total, totalBtw etc) and avoid O(n) iteration if you modify your add / remove code diligently. If you still want to iterate on entire cart from code maintainability aspect, you can introduce one variable to read cart state, and cache these values to avoid unnecessary iteration on entire cart until cart state changes.
  4. Supporting @konijn point - if you want to store price in localStorage, ensure that you have server side logic to override price and other sensitive cart item details to avoid hacks there. Try showing up overridden price to user as well ( by syncning with server ) when you user is about to pay. That ways you will lead to a better user experience and less dissatisfaction among users.

Couple of more points -

  1. Why have you given time delay of 750 to your timeout function. It might cause multiple duplicate items (same items might show up as different individual row instead of incremented count within one row) on UI if user keeps clicking on "add item" (which triggers addToCart scope function) frequently.
  2. Why are you keeping reference of localStorage at scope level? Are you using it anywhere in scope ? (referring to this - $scope.localStorage = $localStorage)
  3. You can improve time complexities of following scope methods (itemWithoutBtw, total, totalBtw etc) and avoid O(n) iteration if you modify your add / remove code diligently. If you still want to iterate on entire cart from code maintainability aspect, you can introduce one variable to read cart state, and cache these values to avoid unnecessary iteration on entire cart until cart state changes.

$scope.itemWithoutBtw = 0;
$scope.addToCart = function(index, title, desc, price, timeout) {
  // Same code as of above, removing this to save space here, Below I wrote for single quantity case. You can write similar logic if item already exists.
  if (!found) {
    $timeout(function() {
        $localStorage.items.push(angular.extend({
        id: index,
        title: title,
        quantity: 1,
        price: price}, index))
        // Below line is added here additionally..
        $scope.itemWithoutBtw += price / 106 * 100;
        $scope.checked = false;
        },750);
    }
};

// You dont need this function and can use scope var directly.
$scope.itemWithoutBtw = function(index) {
    return $scope.itemWithoutBtw;
};

  1. Supporting @konijn point - if you want to store price in localStorage, ensure that you have server side logic to override price and other sensitive cart item details to avoid hacks there. Try showing up overridden price to user as well ( by syncning with server ) when you user is about to pay. That ways you will lead to a better user experience and less dissatisfaction among users.
Source Link

Couple of more points -

  1. Why have you given time delay of 750 to your timeout function. It might cause multiple duplicate items (same items might show up as different individual row instead of incremented count within one row) on UI if user keeps clicking on "add item" (which triggers addToCart scope function) frequently.
  2. Why are you keeping reference of localStorage at scope level? Are you using it anywhere in scope ? (referring to this - $scope.localStorage = $localStorage)
  3. You can improve time complexities of following scope methods (itemWithoutBtw, total, totalBtw etc) and avoid O(n) iteration if you modify your add / remove code diligently. If you still want to iterate on entire cart from code maintainability aspect, you can introduce one variable to read cart state, and cache these values to avoid unnecessary iteration on entire cart until cart state changes.
  4. Supporting @konijn point - if you want to store price in localStorage, ensure that you have server side logic to override price and other sensitive cart item details to avoid hacks there. Try showing up overridden price to user as well ( by syncning with server ) when you user is about to pay. That ways you will lead to a better user experience and less dissatisfaction among users.