@ref21

Ревью кода. Как можна улучшить этот код?

Изучаю javascript.
Написал фильтр для галереи фото. Напишите, пожалуйста, свое мнение о коде и как его можна улучшить.
Вот код:
const   filterButtonsUl	      =  document.getElementById('filter__tiles'),
	 		filterButtons         =  filterButtonsUl.querySelectorAll('.filter__btn'),
	 		filterableTilesUl     =  document.getElementById('featured__tiles'),
	 		filterableTilesLi     =  filterableTilesUl.querySelectorAll('.featured__photo'),
	 		notFound              =  document.getElementById('n-found'),
	 		showAllBtn            =  document.getElementById('show-all');

	// - Gallery filter 
 	function filteringGallery(buttonId) {

	 	let    thisBtnId = buttonId, // - Save ID selected button
	 		   allTilesVisible = true;	
	 		   newArray  = [];
	 		
	 	console.log(thisBtnId);
	 	function createArray() { // - Create an array from the NodeList
	 		filterableTilesLi.forEach( (item) => {
	 			newArray.push(item);
	 		})
	 	}

	 	function showAllTiles() { // - Show all items
	 		filterableTilesLi.forEach(function(item) {
	 			item.style.display = 'block';
	 		})
	 	}

	 	function checkIdButton() { // - Show all items if selected "ALl" button
	 		if(thisBtnId === 'all') 
	 			showAllTiles(); 
	 	}
	 	   
	 	function addDisplay() { // - Add a property display to items from the array
	 		newArray.forEach((item) => {
	 			if(item.classList.contains(thisBtnId)) {
		 			item.style.display = 'block'; // - Show filterable elements
		 		} else {
		 			item.style.display = 'none'; // - Hide not eligible elements 
		 		}
	 		})
	 	}

	 	function check() { // - Checks to hide all the items
	 		let allHidden = false;
	 		
	 		function dspNone(item) { // - Check every item of the array to display: none
	 			return item.style.display === 'none'
	 		}

	 		if(newArray.every(dspNone)) 
	 			allHidden = true;
	 		else 
	 			allHidden = false;


	 		if(allHidden === true)  
	 			notFound.classList.add('visible'); //- If all the items are hidden, show message text
	 		
	 		else 
	 			notFound.classList.remove('visible');
	 		
	 	}

	 	
	 	
	 	if(thisBtnId === 'all') {
	 		showAllTiles(); 
	 	} else {
	 		createArray();
	 		addDisplay();  
	 		check();	
	 		
	 	}
	 	

	 	
	};


	filterButtonsUl.onclick = function(event) {
		let target = event.target;
		console.log(target)

		if(target.className !== 'filter__btn') return;

		filteringGallery(target.id);
	}
 

	showAllBtn.addEventListener('click', function() {
		filterableTilesLi.forEach(function(item) {
	 		item.style.display = 'block';
	 	});
	 	notFound.classList.remove('visible');
	})
  • Вопрос задан
  • 146 просмотров
Решения вопроса 2
Stalker_RED
@Stalker_RED
showAllTiles() { // - Show all items
checkIdButton() { // - Show all items if selected "ALl" button

Возможно стоит сразу нормально назвать (или переименовать)?
С отступами какая-то фигня. Настрой редактор.
Ну и можешь вставить свой код сюда, например, там будут подсказки что исправлять.
Metrics
There are 13 functions in this file.

Function with the largest signature take 1 arguments, while the median is 1.

Largest function has 13 statements in it, while the median is 1.

The most complex function has a cyclomatic complexity value of 3 while the median is 1.

Eight warnings
19	Missing semicolon.
25	Missing semicolon.
40	Missing semicolon.
47	Missing semicolon.
77	Unnecessary semicolon.
82	Missing semicolon.
87	Missing semicolon.
95	Missing semicolon.
One undefined variable
13	newArray
18	newArray
34	newArray
50	newArray
Three unused variables
12	allTilesVisible
28	checkIdButton
2	filterButtons


И да, хинтеры и линтеры могут встраиваться во многие редакторы и IDE.
Ответ написан
Комментировать
Добрый день! Сразу бросается в глаза названия переменных и функций;

newArray - не отображает сути
check() - не отображает сути
notFound - что не найдено ?
thisBtnId - зачем this ?

вместо if else можно писать тернарные операторы ;
вместо function () {} стрелочные функции () => {}

вместо if(allHidden === true) просто if(allHidden);

зачем эта строчка if(target.className !== 'filter__btn') return ?

можно так :
if(target.className === 'filter__btn') {
      filteringGallery(target.id);
      };
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы