Как сократить такое условие?

Question

[Shimpanze]

Добрый день!

Как сократить такое условие?

if (
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "h1" ) &&
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "h2" ) &&
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "h3" ) &&
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "h4" ) &&
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "h5" ) &&
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "h6" ) &&
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "p" ) &&
  ( element.previousSibling !== null && element.previousSibling.nodeName.toLowerCase() !== "img" )
) {}

Answer 1

[mad_god]

element.previousSibling !== null
вынести во внешнее условие,

element.previousSibling.nodeName.toLowerCase() сделать один раз перед проверкой
остальное - внутри

"h1", "h2", "h3", "h4", "h6", "p", "img"

можно сделать списком и проверять на вхождение в список

Comments

[Shimpanze]

Сделал, но почему-то не работает

// Элементы идут именно списком. Не хочется плодить переменную еще и под массив...
var names = "h1, h2, h3, h4, h5, h6, p, img";
if ( element.previousSibling !== null ) {
  // ... поэтому массив создаю прямо здесь
  // (проверил в консоли - к массиву претензий нет. Создается)
  if ( ![...names.split(",\s*")].includes( element.previousSibling.nodeName.toLowerCase() ) ) {

...и нифига не хочет их перебирать. Дальше первого не идет.

Answer 2

[Дмитрий Дерепко]

if (element.previousSibling !== null) {
    const lowerName = element.previousSibling.nodeName.toLowerCase();
    const tags = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'img'];    
    if (!tags.includes(lowerName) {
        // ...
    }
}

Или так:

const tags = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'img'];
if (element.previousSibling !== null && !tags.includes(element.previousSibling.nodeName.toLowerCase())) {    
    // ...
}

Но здесь уже вопрос читаемости. По мне 1 вариант лучше 2.

Comments

[Shimpanze]

По каким-то причинам, второй вариант перестает работать на первом h1 и далее ни чего не проверяет

[edward_freedom]

Shimpanze, научись пользоваться дебагом и какие то причины станут явными причинами

[Дмитрий Дерепко]

Shimpanze, что значит "перестает работать на первом h1"?

[Shimpanze]

Ну где тут ошибка? Все ведь правильно написано...

// Элементы идут именно списком. Не хочется плодить переменную еще и под массив...
var names = "h1, h2, h3, h4, h5, h6, p, img";
if ( element.previousSibling !== null ) {
  // ... поэтому массив создаю прямо здесь
  // (проверил в консоли - к массиву претензий нет. Создается)
  if ( ![...names.split(",\s*")].includes( element.previousSibling.nodeName.toLowerCase() ) ) {

...и нифига не хочет их перебирать. Дальше первого не идет.

[Дмитрий Дерепко]

Shimpanze, вы когда воспроизводите ответы, воспроизводите их сначала в точности, как советуют, а потом уже всякие split() делайте.

Answer 3

[Владимир Солнцев]

Наверное можно и без сравнения с null, ведь условие преобразуется в логический тип, а null в js преобразуется в false:

if (element.previousSibling) { 
 // ... остальной код Дмитрия Дерепко
}

// Или без вложенных циклов
// сначала проверить:  если у элемента нет предшествующего элемента
if (!element.previousSibling) { 
  return element;  
  // вернуть элемент, чтобы другая функция что-то с ним могла сделать. 
  // Или вернуть false, не суть. Важно просто обработать сценарий, когда элемент 
  // идет первым в родителе. Можно просто выйти из функции.
}
// Дальше, если предшественник есть, делаем как в коде Дмитрия
const targetTags = ['h1', 'h2', ...],
    lowerName = element.previousSibling.nodeName.toLowerCase();

if (!targetTags.includes(lowerName)) { 
  // do samething
}

// Кстати, можно targetTags передавать сразу аргументом в функцию, чтобы код был чуть 
// более универсальным.
function doSomethingEpic(element, targetTargs) {
 // code
}

const someElem = document.querySelector('.epicElement');
const tags = ['h1', 'h2', ...],
doSomethingEpic(someElement, tags);

Важно: метод includes не поддерживается в IE, полифилл есть на сайте MDN

Comments

[Shimpanze]

Ну где тут ошибка? Все ведь правильно написано...

// Элементы идут именно списком. Не хочется плодить переменную еще и под массив...
var names = "h1, h2, h3, h4, h5, h6, p, img";
if ( element.previousSibling !== null ) {
  // ... поэтому массив создаю прямо здесь
  // (проверил в консоли - к массиву претензий нет. Создается)
  if ( ![...names.split(",\s*")].includes( element.previousSibling.nodeName.toLowerCase() ) ) {

...и нифига не хочет их перебирать. Дальше первого не идет.

[Владимир Солнцев]

Shimpanze, вот так условия проходят на этой странице. Открываете консоль прям на этой странице, вводите код, кликаете ввод, далее вызываете функцию, попробуйте.

var el = document.querySelector('.layout__body'); 
var names = "h1, h2, h3, h4, h5, h6, p, img";

function doSomeEpic (element, tags) {
	if (element.previousSibling !== null) { 
		console.log('PrevSib: ' + element.previousSibling.nodeName);
		
		if (!tags.split(', ').includes(element.previousSibling.nodeName.toLowerCase())) {
			console.log('Ура, заработало');
		}
	}
}

// дальше вызвать функцию doSomethingEpic(el, names);

Не совсем понимаю, для чего вы используете сприд-оператор, если это какая-то оптимизация, то я не в курсе).
Но и такой вариант тоже работает, попробуйте заменить часть tags.split(',') на [...tags.split(',')]

На счет плодить переменные под массив - в текущих реалиях это экономия на спичках. Хотя, скорее даже на вдохах)
В общем лишнее это, на мой взгляд, но ведь на вкус и цвет все фломастеры разные)
Можно просто после того, как массив names будет не нужен, очистить ссылку names = null;