HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Mini shopping cart for a site on MODX

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
minicartsiteformodxshopping

Problem

Please help in improving the code. I would like to hear comments on syntax and an approach to development. This is my first experience in writing a simple application for the site.

```
window.onload = function() {

/* инициализация таблицы-прайса
Вначале проверяем наличие таблицы-прайса на странице.
Класс таблицы с прайсом должен быть => table-price.
Подразумевается такая структура колонок таблицы прайса:
| Артикул | Наименование | Стоимость |

*/

// проверяем наличие прайса на странице
function priceOnPage(){
if (document.getElementsByTagName('table') !== undefined) {
var tables = document.getElementsByTagName('table');

for (var i=0, tablesLength = tables.length; iДобавить в корзину';
continue;
}

item[j].setAttribute('class', option[j]);
};
};

/ Набор стандартных кроссбраузерных функций /

// кроссбраузерная установка обработчиков
function addEvent(el, type, handler) {
if (el.addEventListener) {
/ Chrome, Mozilla, Opera, Safari /
el.addEventListener(type, handler);
} else {
/ IE /
elem.attachEvent('on' +type, function() {
handler.call(elem);
});
};
return false;
};

// получение/запись данных в LocalStorage
function getCartData(item) {
if (item) {
// если передаются даныне, то записываем...
localStorage.setItem('cart', JSON.stringify(item));
return false;
} else {
// ... либо получаем
return JSON.parse(localStorage.getItem('cart'));
};
};

// проверка объета на пустоту
function isEmpty(obj) {
var count = 0;
for (var key in obj) {
count++;
};

if (count>0) {
return false;
};
return true;
};

/ Основной функционал /

// если LS

Solution


  • Coments



Don't use Russian (or any other langugage except English) in comments. Also, it's a good idea to not comment your code much in obvious cases, e.g.

// Удаляем товар из корзины
function deleteItem() {


  • priceOnPage



Why not use getElementById? It can be slow to search throw all tables in DOM. Also, you're searching for it twice.

  • isEmpty



You can just use something way more simpler:

function isEmpty(obj) {
    for (var _ in obj) {
        return false;
    }
    return true;
}


  • Don't mix code and functions



Just split them: all functions first, then code.

  • ; after block statements maybe omitted



I'll try to add more later.

Code Snippets

// Удаляем товар из корзины
function deleteItem() {
function isEmpty(obj) {
    for (var _ in obj) {
        return false;
    }
    return true;
}

Context

StackExchange Code Review Q#100914, answer score: 2

Revisions (0)

No revisions yet.