Функция для подсветки отмеченных элементов


Исходный код

var newstr = null;
		var lastnewstr = null;
		var newclass = null;
		var lastclass = null;
		var newstr2 = null;
		var lastnewstr2 = null;
		var newclass2 = null;
		var lastclass2 = null;
		var newstr3 = null;
		var lastnewstr3 = null;
		var newclass3 = null;
		var lastclass3 = null;
		
function check(s, i)
{
		p1 = document.getElementById("id_1_"+i);
		p2 = document.getElementById("id_2_"+i);
	if (s!=lastnewstr)
	{
		newclass2 = p1.className;
		newclass3 = p2.className;
		newclass = s.className;

		s.className="check";
		p1.className="viz";
		p2.className="viz";

		if (lastnewstr!=null)
		{
			lastnewstr.className=lastclass;
			lastnewstr2.className=lastclass2;
			lastnewstr3.className=lastclass3;
		}
	}
	lastnewstr = s;
	lastnewstr2 = p1;
	lastnewstr3 = p2;
	lastclass = newclass;
	lastclass2 = newclass2;
	lastclass3 = newclass3;
}

Что не так в этом коде

По реализации функции совершенно непонятно, что в ней происходит; невозможно понять её предназначение.

Причина непонятности:

  • непонятно, что означают все эти переменные (названия слабо указывают на предназначение переменных или не указывают вовсе)
  • последовательность операций не позволяет увидеть алгоритм
  • перемещения информации не позволяют увидеть за ними алгоритм

Вариант рефакторинга кода

После напряжённого анализа кода выяснилось, что алгоритм подразумевает присвоение новых классов элементам при пометке (check), при этом старые классы запоминаются, чтобы восстановить их в следующий раз, когда будет помечен другой элемент.

var currentRow, currentRow2, currentRow3,
	previousClassName, previousClassName2, previousClassName;
		
function check(newCurrentRow, i)
{
	if (newCurrentRow != currentRow)
	{
		if (currentRow)
		{
			// восстановили предыдущие классы у текущих элементов
			currentRow.className  = previousClassName;
			currentRow2.className = previousClassName2;
			currentRow3.className = previousClassName3;
		}

		// заменили текущие элементы на новые
		currentRow  = newCurrentRow;
		currentRow2 = document.getElementById("id_1_"+i);
		currentRow3 = document.getElementById("id_2_"+i);

		// запомнили старые классы элементов
		previousClassName  = currentRow.className;
		previousClassName2 = currentRow2.className;
		previousClassName3 = currentRow3.className;

		// присвоили новые классы элементам
		currentRow.className = "check";
		currentRow2.className = "viz";
		currentRow3.className = "viz";
	}
}

В новом варианте более ясная последовательность операций и более говорящие названия переменных, объясняющие смысл хранимых величин.

Убраны переменные и операции, оказавшиеся лишними.

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

К счастью, сейчас такие знания входят в азы вёрстки.

Современный вариант рефакторинга

Такой хитрый алгоритм мог понадобиться в те далёкие времена, когда у элементов DOM ещё не было свойства classList, в который можно добавлять и удалять необходимые классы, не трогая остальные.

Вероятно, автор был не знаком с jQuery или он был запрещён на проекте.

В современных условиях, несомненно, нужно пользоваться методами add и remove для свойства classList.

Теория

  • Ребусы в коде
  • Названия переменных