Govnokodu.Net

Пример рефакторинга «извлечение метода» при поиске и выводе типа пакета

Исходный код

while (!bFound && j < enmMessages)
{
	if (!strcmp(str.operatorconstchar * (), strPacketName[j]))
	{
		iPacketType = j;
		bFound = true;
		TRACE(" of type %s\n",strPacketName[j]);
		strcat(strOut," of type ");
		strcat(strOut, strPacketName[j]);
		break;
	}
	j++;
}

Какой рефакторинг применить к исходному коду

  1. В этом цикле смешаны две ответственности: найти подходящий элемент и предпринять соответствующие действия при успешном поиске. Рекомендую рефакторинг «извлечение метода», извлечённый метод будет искать пакет с именем str.
  2. Для обхода списка пакетов применён цикл while, тогда как по смыслу больше подходит for.
  3. Стоит избавиться от избыточной переменной bFound.
  4. Для получения указателя на массив символов, из которых состоит строка str, применена громоздкая малопонятная конструкция .operator const char * (), в то время как более ясный способ получить указатель — применить вызов метода c_str().
  5. Для сравнения строк применяется сишная функция strcmp(), хотя для сравнения с массивом символов можно использоать штатный метод compare() самой строки.
  6. Названия переменных не согласованы между собой: массив пакетов назван strPacketName — ожидается, что в нём названия пакетов, а номер найденного пакета — iPacketType, тип пакета.
  7. В идеале массив пакетов должен быть оформлен в виде самостоятельного класса, чтобы отделить код по его обслуживанию от кода, который получает выгоду.

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

int findPacket(const string& packetName) {
	for (int i = 0; i < enmMessages; i++) {
		if (!packetName.compare(strPacketName[i])) {
			return i;
		}
	}
	return -1;
}
foundPacket = findPacket(str);
if (foundPacket >= 0) {
	TRACE(" of type %s\n",strPacketName[foundPacket]);
	strcat(strOut," of type ");
	strcat(strOut, strPacketName[foundPacket]);
}

Теория

Сергей Галанин