Генерация таблицы из JSON

[Lemme]

tysonfury2015, надеюсь, когда нибудь ты отойдешь от той "хрени", которую ты употребляешь, ибо такую ересь можно "нести" только в нетрезвом состоянии.

По делу:

tsigel, попробовал учесть все ваши замечания, кроме методов в 3-4 строки =). Ну и немного изменил хранилище данных (точнее его наполнение), ибо столкнулся с проблемой его переполнения.

class Tysonfury2015_idi_mimo{
	/**
	 * @constructor
	 * @param  {Object} config{url: 'path/to/json', perPage: Integer, output: { container: 'element class/id'}}
	 */
	constructor(config){
		this.config = config;
		this.init();
		this.storageToken = 'Data__' + Math.round(new Date().getTime() / 1000);
	}
	/**
	 * Получет данные в формате JSON по адресу this.config.url
	 * 
	 * @param  {Callback}
	 */
	getData(success){
		fetch(this.config.url)
			.then((response)=>{
				if (response.status !== 200){
					console.error('Response error! Status - '+response.status);
					return;
				}
				response.json().then((response)=>{
					success(response);
				});
			})
		.catch((error)=>{
			console.error(error);
		});
	}
	/**
	 * Рабиваем массив с данными по страницам 
	 * 
	 * @param  {Array}
	 */
	splitDataByPages(data){
		let	pages		= [],
			header  	= data.shift(),
			numPages	= Math.ceil(data.length / this.config.perPage);


		for (let i = 0; i < numPages; i++){
			pages[i] = [];
			for(let j = 0; j < this.config.perPage; j++){
				if (data.length === 0) break;
				pages[i].push(data.shift());
			}
		}

		this.putDataToStorage({header, pages});
	}
	/**
	 * Добавляет данные в sessionStorage
	 * 
	 * @param data
	 */
	putDataToStorage(data){
		sessionStorage.setItem(this.storageToken, JSON.stringify(data));
	}
	/**
	 * Возвращает данные из sessionStorage
	 * 
	 * @return data
	 */
	getDataFromStorage(){
		return JSON.parse(sessionStorage.getItem(this.storageToken));
	}
	/**
	 * Удаляет все данные из sessionStorage , которые относятся к этому классу
	 */
	clearStorage(){
		Object.keys(sessionStorage)
		.filter((key)=>{
			return /^Data__\d+$/.test(key);
		})
		.forEach((key)=>{
			sessionStorage.removeItem(key);
		});
	}
	/**
	 * Генерирует основную структуру таблицы
	 * Таблица (table), заголовок(thead)
	 */
	makeTable(){
		let table	= document.createElement('table'),
			thead	= document.createElement('thead'),
			tr		= document.createElement('tr'),
			header	= this.getDataFromStorage().header;

		for (let item of header){
			let td = document.createElement('td');
			td.innerHTML = item.title;
			tr.appendChild(td);
		}

		thead.appendChild(tr);
		table.appendChild(thead);

		this.table = table;
	}
	/**
	 * Генерирует таблицу (tbody) по странице num
	 * 
	 * @param {Number} num - текущая страница
	 */
	setPage(num = 1){
		num = num < 1 || typeof num !== 'number' ? 1 : num >= this.getDataFromStorage().pages.length ? this.getDataFromStorage().pages.length : num;

		let page 	= this.getDataFromStorage().pages[num - 1],
			tbody 	= document.querySelector(this.config.output.container+' table tbody');
			
		if (!tbody){
			tbody = document.createElement('tbody');
		}
		else {
			tbody.innerHTML = '';
		}
		page.forEach((row)=>{
			let tr = document.createElement('tr');
			row.forEach((cell)=>{
				let td = document.createElement('td');
				td.innerHTML = cell;
				tr.appendChild(td);
			});
			tbody.appendChild(tr);
		});
		this.table.appendChild(tbody);
	}
	/**
	 * Генерирует структуру постраничной навигации
	 */
	makePagination(){
		let numPages = this.getDataFromStorage().pages.length;

		if (numPages <= 1) return;

		let items = document.createElement('ul');

		for (let i = 0; i < numPages; i++){
			let item = document.createElement('li');
			item.innerHTML = i+1;
			items.appendChild(item);
		}

		items.firstChild.classList.add('active');
		this.pagination = items;
	}
	/**
	 * Устанавливает слушателей событий
	 */
	setListeners(){
		if (this.pagination){
			this.pagination.addEventListener('click', (e)=>{
				if (e.target.tagName !== 'LI' || e.target.classList.contains('active')) return false;

				document.querySelector(this.config.output.container+' ul .active').classList.remove('active');
				e.target.classList.add('active');

				this.setPage(+e.target.innerHTML);
			});
		}
	}
	/**
	 *  Последовательно вызывает методоы необходимые для генерации, обработки таблицы и навигации по ней
	 */
	init(){
		this.clearStorage();
		this.getData((data)=>{
			this.splitDataByPages(data);
			this.makeTable();
			this.setPage(1);
			this.makePagination();
			
			let container = document.querySelector(this.config.output.container);
			container.appendChild(this.table);
			if (this.pagination){
				container.appendChild(this.pagination);
			}
			
			this.setListeners();
		});
	}
}

Если я Вас правильно понял и улучшил код, то, если не сложно, подскажите, что еще можно исправить/улучшить?

[tsigel]

Lemme,
Судя по коду вы не используете ide (по крайней мере при reformatCode не осталось бы выравненных присвоений). Это не плохо но с ide удонее. Мне нравится этот гайд по стилю кода.

Стало лучше. Мне не ясно зачем тут localStorage? потому как если данные тянутся с сервера то их должно быть не много. И они должны тянуться постронично. На сколько я понимаю это просто для себя. Ок.

clearStorage - удалить ключи можно в 1 проход.
getData - зачем колбек если у вас есть промис? Возвращайте промис, красиво же!

Вы много используете красивых итераторов, но по данным бежите обчсным фором. Почему? В коде должно быть единообразие по максимому. Допустимы сильные отличия в пользу производительности там где она нужна.

по возможности действия метода должны нести описательный характер, например в setPage вы проверяете валидность индекса, но намного лучше это вынести в отдельный метод это в 10 раз повысит читабельность метода

Я на практике убедился что много однострочных методов - это хорошо, и условия тоже хорошо бывыносить. Это позволяет не вникать в скобки и прочее, а сразу понять суть проверки по названию метода.

[nerv_]

Lemme, тебя не смущает, что твоя таблица (Table) работает со страницами?

splitByPages
switchPage
pagination



Это из серии

car.cookBorsch();
rabbit.fly();
window.swim()

[Lemme]

Судя по коду вы не используете ide (по крайней мере при reformatCode не осталось бы выравненных присвоений). Это не плохо но с ide удонее. Мне нравится этот гайд по стилю кода.

Нет, ide не использую, использую sublime text. Посмотрел гайд, очень ко многому придется привыкать, но, думаю, постепенно буду соблюдать=)

Стало лучше. Мне не ясно зачем тут localStorage? потому как если данные тянутся с сервера то их должно быть не много. И они должны тянуться постронично. На сколько я понимаю это просто для себя. Ок.

Задумка в том, чтоб получить данные сервера, а ковырять уже на клиенте, localStorage для того, что б дальше работать с данными.

clearStorage - удалить ключи можно в 1 проход.

так?

for (let key in sessionStorage){
	if (/^Data__\d+$/.test(key)){
		sessionStorage.removeItem(key);
	}
}

getData - зачем колбек если у вас есть промис? Возвращайте промис, красиво же!

А вот этого, к сожалению, не понял

Вы много используете красивых итераторов, но по данным бежите обчсным фором. Почему? В коде должно быть единообразие по максимому. Допустимы сильные отличия в пользу производительности там где она нужна.

Потому, что тут циклы работают с цифрами.

по возможности действия метода должны нести описательный характер, например в setPage вы проверяете валидность индекса, но намного лучше это вынести в отдельный метод это в 10 раз повысит читабельность метода

Я на практике убедился что много однострочных методов - это хорошо, и условия тоже хорошо бывыносить. Это позволяет не вникать в скобки и прочее, а сразу понять суть проверки по названию метода.

Возьму на заметку, спасибо.

[Lemme]

car.cookBorsch();
rabbit.fly();
window.swim()

То все мелочи

[tsigel]

Потому, что тут циклы работают с цифрами.

Это отмазка. Вы автор кода и вы создаете формат, можете написать так чтобы не было цифр и были массивы.

так?
for (let key in sessionStorage){
    if (/^Data__\d+$/.test(key)){
        sessionStorage.removeItem(key);
    }
}

Например, или так:

var reg = /^Data__\d+$/;
Object.keys(sessionStorage).forEach((key) => {
   if (reg.test(key)) {
      sessionStorage.removeItem(key);
   }
});

Вынесение объявления рег выражения позволит не создавать его при итерации каждого имени.

Методы не просто по возможности должны носить описательный характер. Всякому дроблению должен быть конец
Не знаю правильно или нет, но мне очень нравится разделять методы которые должны выполнять логику от тех что несут реализацию. И по возможности их не мешать.

Ну на примере того же setPage (мне нравится typescript пример будет на нем)

public setPage(index:number):void {
   if (this.hasPage(index)) {
      this.changeCurrentPage(index);
      this.render();
   }
}

public render():void {
   this.clear();
   this.createBody();
}

private changeCurrentPage(index:number):void {
   this.currentPage = index;
}

Конечно многие скажут что удалять тело таблицы и потом его создавать - накладная операция, а таблица должна быть быстра, так что тут это не совсем удачный пример, НО метод clear - нужная штука, он может быть переиспользован в удалении таблице или изменении данных от сервера и прочее, и отдельный метод на создание данных тоже нужен. дробление методов позволяет увеличить переиспользуемость написанного кода.

[tsigel]

Lemme,
Оптимизации и красивости кода должны гармонировать, не стоит писать лишний проход по массиву если его можно не писать и код будет красив, но при этом если массив заведомо мал, а метод используется редко и за один проход выглядит не красиво - не надо пытаться свалить все в 1 кучу (ну то есть не надо пытаться отфильтровать и отмапить и отсортировать в 1 проход массив из 5 элементов, профита вы не увидите).

[Lemme]

tsigel, спасибо за помощь=)

[tsigel]

Lemme,
Перебирая объекты не забывайте про hasOwnProperty

А вот этого, к сожалению, не понял

Зачем getData принимает коллбек? пусть возвращает промис!

getData() {
   var promise = fetch(this.config.url);
   promise.catch(...);
   returm promise.then(...);
}

init() {
  this.getData().then((data) => {
    ...
  });
}

[Lemme]

getData() {
   var promise = fetch(this.config.url);
   promise.catch(...);
   returm promise.then(...);
}

init() {
  this.getData().then((data) => {
    ...
  });
}

А вот это красиво =)