Govnokodu.Net

Пример рефакторинга «извлечение функции» и улучшение имён переменных при инкременте 36-ричного числа

Исходный код

function levelIncrease($lvl)
{
	for($i = 2, $ng = 0; $i >= 0; $i--) {
		$g = ord($lvl[$i]);
		if($i == 2)
			++$g;
		$g += $ng;
		$ng = 0;
		if($g > 57 && $g < 97)
			$g = 97;
		elseif($g > 122) {
			$diff = $g - 122;
			$g = 48 + $diff - 1;
			$ng = ceil($diff / 36);
		}
		$lvl[$i] = chr($g);
		if($ng == 0)
			break;
	}
	return $lvl;
}

echo levelIncrease('aaa');   // aab

Что не так в исходном коде

Если в названии функции присутствует глагол, то он должен стоять на первом месте: надо что-то типа increaseLevel или incrementLevel.

Однако с моей точки зрения эта функция не функция-команда, а функция-запрос, поэтому глагол здесь не особо нужен. Можно nextLevel.

Алгоритм инкремента опирается не на фактическую длинну полученного кода, а на допущение, что она всегда будет равна трём. Не всегда такие допущения долго остаются в силе, поэтому есть смысл опираться не на них, а на факты.

В алгоритме смешаны три разных знания: как инкрементировать единичный разряд кода, как выполнить перенос разряда и как инкрементировать весь код в целом.

Непонятно, для чего сделано так, что перенос разряда может быть больше единицы.

Неговорящие названия переменных затрудняют чтение алгоритма.

Магические литералы тоже ясности не добавляют.

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

function nextLevel($level) 
{
	for ($i = length($level) - 1, $carry = 1; $i--) {
		list ($level[$i], $carry) = nextDigit($level[$i], $carry);
		if ($carry == 0) break;
	}
	return $level;
}

const CODE_0 = 48;
const CODE_9 = 57;
const CODE_A = 97;
const CODE_Z = 122;
function nextDigit($digit, $carry) {
	// перенос может быть только 0 или 1
  $digit_code = ord($digit);
	$digit_code += $carry;
	if ($digit_code > CODE_0 && $digit_code < CODE_A) {
		return ['a', 0];
	}
	elseif ($digit_code > CODE_Z) {
		return ['z', 1];
	}
	return [ord($digit_code), 0];
}

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

function nextLevel($level)
{
	$level10 = base_convert($level, 36, 10);
	$level10++;
	return base_convert($level10, 10, 36);
}

Теория

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