Рефакторинг кода JS

531

29 декабря 2016, 09:30

Делаю динамический поиск, т.е. добавляю и удаляю элементы (в таблице) на лету. Можно ли как-то сократить код? data_parse - ассоциативный массив, полученный из сервера.

let data_parse = JSON.parse(data);
console.log(JSON.parse(data));
$('#listDataEmployee').remove();
let el_tbody = document.createElement('tbody');
for (let i = 0; i < data_parse.length; i++) {
    let tagTrDataEmployee = document.createElement('tr');
    let tagTd_id = document.createElement('td');
    tagTd_id.innerHTML = data_parse[i]['id'];
    tagTrDataEmployee.append(tagTd_id);
    let tagTd_email = document.createElement('td');
    tagTd_email.innerHTML = data_parse[i]['email'];
    tagTrDataEmployee.append(tagTd_email);
    let tagTd_fio = document.createElement('td');
    tagTd_fio.innerHTML = data_parse[i]['FIO'];
    tagTrDataEmployee.append(tagTd_fio);
    let tagTd_phone = document.createElement('td');
    tagTd_phone.innerHTML = data_parse[i]['phone'];
    tagTrDataEmployee.append(tagTd_phone);
    let tagTd_role = document.createElement('td');
    tagTd_role.innerHTML = data_parse[i]['role'];
    tagTrDataEmployee.append(tagTd_fio);
    el_tbody.append(tagTrDataEmployee);
}
$('.listNewDataEmployee').append(el_tbody);

Answer 1

Полагаю, на самом деле ты хочешь что-то такое:

var data = `[ 
  {"id": 1, "email": "first@mail.ru", "FIO":"First Firstov", "phone": "+7 (111) 111 11 11", "role": "admin"}, 
  {"id": 2, "email": "second@mail.ru", "FIO":"Second Secondov", "phone":"+7 (222) 222 22 22", "role": "user"}, 
  {"id": 3, "email": "xss@mail.ru", "FIO":"<img src='http://void' onerror='alert(1)'>", "phone":"+7 (333) 333 33 33", "role": "test"} 
]`; 
 
data = JSON.parse(data); 
 
$('#listDataEmployee').replaceWith( 
  data.map(d => $("<tr>").append([ 
    $("<td>").text(d.id), 
    $("<td>").text(d.email), 
    $("<td>").text(d.FIO), 
    $("<td>").text(d.phone), 
    $("<td>").text(d.role), 
])));

table, tr, td, th { border-collapse: collapse; border: 1px solid; }

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script> 
<table> 
  <thead> 
    <tr> 
      <th>ID 
      <th>Email 
      <th>FIO 
      <th>Phone 
      <th>Role 
    </tr> 
  <tbody id="listDataEmployee"> 
</table>

Answer 2

Первое что приходит в голову - объединить однотипные условия как-то так:

// Перед циклом, чтобы не создавать этот объект в каждой итерации
let columnsList = [
    'id', 
    'email', 
    'FIO',
    'phone',
    'role'
];
// Немножко кода пропущено
// Внутри тела цикла
columnsList.forEach(columnName => {
    var tdEl = document.createElement('td');
        tdEl.innerHTML = data_parse[i][columnName];
        tagTrDataEmployee.append(tdEl);
});

На более сложные оптимизации я в 2 часа ночи не способен)
Могу еще сказать если таблица реально очень большая то быстрее склеивать строку, а потом уже делать из нее DOM элементы, но здесь - роли не играет.

Еще можно не хардкодить лист колонок а брать прям из объекта все что приехало:

let columnsList = Object.keys(data_parse[i]);

(Как верно заметил Qwertiy у Object.keys может вернуть ключи не в том порядке что у вас в шапке, и это надо иметь в виду)

UPD: посмотрел с утра внимательнее. Вы используете jQuery, что значительно упрощает описанную выше оптимизацию.
Можно написать что-то в духе $('<div>test</div>'), то есть скормить jQuery кусок html написанный в виде строки, и он разберет это сам. В результате кусок создающий строку можно записать совсем просто:

let rowString = `<tr>`;
columnsList.forEach(columnName => {
     rowString += `<td>${data_parse[i][columnName]}</td>`;
});
rowString +=`</tr>`;
let rowEl = $(rowString);

(На работоспособность я проверять поленился, но думаю идея понятна).
Внимание: как верно заметил Qwertiy что если данные которые Вы показываете вводятся юзером и не валидируется - есть шанс пропустить xss.
Но если так показываются какие-то заранее валидные данные, то можно и так.

Почему это имеет смысл? Дело в том, что операции с js строками сами - очень быстрые. Операции по изменению в DOM - значительно медленнее. Вызовы каких-то браузерных API за пределами js движка - в целом медленнее.

Да, обратите внимание что я использую в коде ES6-конструкции (=> - стрелочные функции, `` - шаблонные строки), но так как у вас уже есть let, я предположил что с этим проблем нет.