Как более красиво/правильно написать этот код?

Question

[Дмитрий]

$('.folio-nav_list li').click(function(e) {
		$('.folio-nav_list li').removeClass('active');
		$('.folio-nav_list li').removeClass('no-active');
		$(this).addClass('active');
		$('.folio-nav_list li').addClass('no-active');
		$(this).removeClass('no-active');
	});

Comments

[Михаил Плюснин]

А что тут должно произойти? Просто зачем столько всего?

[Дмитрий]

Есть три пункта в навигации. У первого по дефолту в HTML коде указан класс active, что бы при загрузке страницы он там был. Он присваивает одни стили. У остальных по дефолту класс no-active, который присваивает другие стили. Нужно что бы по клику на пункт к нему добавлялся класс active, а остальным пунктам класс no-active.

[Михаил Плюснин]

Дак лучше наверное ставить просто пункту меню класс .active и так понятно, что активным может быть только один, а в коде при клике у всех убираем .active и после добавляем active по тому который кликнули

[Дмитрий]

Каждый из классов задает отдельные стили. Класс active делает бордер, а класс no-active активный бордер при ховере. Вот

[Михаил Плюснин]

ну дак вы ховер то не включайте на актив :hover:not(.active)

[Дмитрий]

Ваша правда, так не попробовал. Но получилось вот такое. Не очень красиво. Бордеры одинаковой высоты.

Answer 1

[Rou1997]

var all_another = $('.folio-nav_list li');
all_another.removeClass('active');
all_another.addClass('no-active');
$(this).addClass('active');
$(this).removeClass('no-active');

Comments

[Дмитрий]

Собственно, все то же, только в профиль :)

[Rou1997]

Дмитрий: Нет, это совсем не то же, потому что не вызывается функция $(), которая в недрах нескольких вызовов вызывает getElementsByClassName, выполняющую обработку DOM-дерева, которая может быть довольно трудоемкой, а еще повышена читаемость кода, непонятно что вы хотите, и главное зачем, отличный же код.

[Дмитрий]

Ну на счет читаемости я не сильно уверен в вашей правоте, ибо все пять строк показывают простую поочередность выполнения кода. А вот на счет производительности: легко ведь можно и на нативный переписать. Я имел в виду, может можно это каким-то методом сократить, или ещё чего. (А если честно, я не очень понял о "не вызывается функция $(), которая в недрах нескольких вызовов вызывает getElementsByClassName, выполняющую обработку DOM-дерева", хотелось бы чуть подробнее :) )

[Rou1997]

Дмитрий: Вот - не понимаете, а спорите!
На "ванильный" JS очень не легко переписывать все подряд, только в особых случаях, а на вопрос "зачем" вы не ответили, значит ваш случай не такой и все работает абсолютно нормально, что там говорил Дональд Кнут про преждевременную оптимизацию, словом не нужно во всем искать единственный идеал, просто хорошо решайте задачу, в то время, как вы заняты ненужной работой, вы могли бы реализовать что-то еще не реализованное, а вы сейчас ничего полезного не делаете, поэтому у перфекционистов огромные запросы к зарплате и прочим условиями и в итоге вообще не выполненные проекты, перфекционизм - это болезнь психическая, лечитесь, а то будет плохо.

[Дмитрий]

Ну я скорее не от тяжбы к перфекционизму задал этот вопрос, а от желания посмотреть на другие, возможно более правильные, реализации данного кода) Ведь это лишь маленький пример, который может быть осложнен в будущем. Да и вот узнал о методе siblings() – тупо перечитывать подряд всю API немного затруднительно. Но все равно спасибо)

[Rou1997]

Дмитрий: Вот когда будет осложнен, тогда бы и решили проблему, а так вы заняты полнейшей х... ерундой, грубо говоря.
Насчет функции $(), вы сравните свой код и мой и увидите, что вы при каждом обращении к элементам ищете элемент по имени класса, могли бы догадаться что любой поиск в огромном HTML-коде занимает время, а у меня вызов $() всего один, затем создается переменная, это и дает оптимизацию, проще обратиться к переменной, чем искать снова и снова.
Насчет siblings(), учтите что чем больше методов, тем сложнее код, и необязательно она будет работать быстрее, по сути в ней тот же цикл по всем элементам, но еще и с условием (if), на него время нужно.
Насчет "тупо перечитывать подряд всю API" - учитесь пользоваться Гуглом, "загуглили" бы что-то вроде "jquery get all child nodes except this" и нашли бы ответ сразу, вот где оптимизация и упрощение, а не в этой хэ.

[Дмитрий]

Что ж, спасибо за критику. Учту ваши советы.

Answer 2

[Bowen]

var noActiveClassName = 'no-active', activeClassName = 'active';
$('.folio-nav_list li').on('click', function() {
    $(this)
    .removeClass(noActiveClassName)
    .addClass(activeClassName)
    .siblings()
    .removeClass(activeClassName)
    .addClass(noActiveClassName);
});

Comments

[Дмитрий]

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