Govnokodu.Net

Слишком много переменных и операций (длина функции)

Исходный код

	long gr, ri, r1, r2, y, z, z_rev;
	for (i = 2; i < qr_count; i++) {
		step = 0;
 		ri = r[i];
		gr = - mpz_fdiv_ui(lowM, pm[i][step]);
		r1 = gr + ri;
		r2 = gr + pm[i][step] - ri;
		if (r1 < 0) {
			r1 += pm[i][step];
		}
		if (r2 < 0) {
			r2 += pm[i][step];
		}
		while (r1 < (long) Ml2) {
			sieve[r1][i] += 1;
			r1 += pm[i][step];
		}
		while (r2 < (long) Ml2) {
			sieve[r2][i] += 1;
			r2 += pm[i][step];
		}
		for (step = 1; step < degr[i]; step++) {
			gr = - mpz_fdiv_ui(lowM, pm[i][step]);
			y = ri*ri;
			y -= Npm[i][step];
			y = y/( (long) pm[i][step-1]);
			if (y < 0) { y += pm[i][step]; }
			z_rev = (ri<<1) % QR[i];
			//?IOOA?EOO ?OI?AOEO IA UAAEEIE?AIEA
			if (z_rev == 0) { y = 0; }
			else {
				z = 1;	
				while ((z_rev*z % QR[i]) != 1) { z++;}
				y *= z*pm[i][step-1];
				y = y % pm[i][step];
			}
			ri -= y;
			if (ri < 0) { ri += pm[i][step]; }
			r1 = gr+ri;
			r2 = gr+pm[i][step]-ri;
			if (r1 < 0) { r1 += pm[i][step]; }
			if (r2 < 0) { r2 += pm[i][step]; }
			while (r1 < (long) Ml2) {
				sieve[r1][i] += 1;
				r1 += pm[i][step];
			}
			while (r2 < (long) Ml2) {
				sieve[r2][i] += 1;
				r2 +=pm[i][step];
			}
		}
	}

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

Чрезмерная сложность кода

Ничего не могу сказать о корректности и предназначении данного кода (какие-то математические расчёты), но что могу сказать сразу и однозначно — этот код переусложнён.

Что делает его переусложнённым?

Даже в этом фрагменте (а это именно фрагмент) чрезмерное количество переменных и операций.

Входные переменные:

  • qr_count — число
  • r — одномерный массив
  • lowM — видимо, число
  • pm — двумерный массив
  • Ml2 — число
  • degr — одномерный массив
  • Npm — двумерный массив
  • QR — одномерный массив

Рабочие (временные) переменные:

  • gr, ri, r1, r2, y, z, z_rev, i, step — целые числа

Выходные переменные

  • sieve — двумерный массив

Итого, если я не сбился со счёта, тут 18 переменных, а операции я уже даже считать не буду.

А ещё отдельными объектами для мозга являются промежуточные значения переменных, под которые мы новые переменные не отводим. В этом примере это переменная y:

    y = ri*ri;
    y -= Npm[i][step];
    y = y/( (long) pm[i][step-1]);
    if (y < 0) { y += pm[i][step]; }
    z_rev = (ri<<1) % QR[i];
    if (z_rev == 0) { y = 0; }
    else {
        z = 1;	
        while ((z_rev*z % QR[i]) != 1) { z++;}
        y *= z*pm[i][step-1];
        y = y % pm[i][step];
    }

Чтобы уменьшить сложность исходного фрагмента кода, буду применять метод рефакторинга под названием «извлечение метода» [Фаулер].

Глядя на наш пример кода, я вижу в нём группу строк со своим локальным набором входных и промежуточных переменных (сразу превращаю её в функцию):

	function step_sieve(long ri, long i, long step) {
		gr = - mpz_fdiv_ui(lowM, pm[i][step]);
		r1 = gr + ri;
		r2 = gr + pm[i][step] - ri;
		if (r1 < 0) {
			r1 += pm[i][step];
		}
		if (r2 < 0) {
			r2 += pm[i][step];
		}
		while (r1 < (long) Ml2) {
			sieve[r1][i] += 1;
			r1 += pm[i][step];
		}
		while (r2 < (long) Ml2) {
			sieve[r2][i] += 1;
			r2 += pm[i][step];
		}
	}

Нетрудно заметить, что этот же фрагмент кода повторяется и внутри вложенного цикла.

Внутри кода новой функции также вижу самостоятельный набор операций, причём тоже повторяющийся дважды. Проведу выделение функции и здесь:

function step_sieve(long ri, long i, long step) {
    gr = - mpz_fdiv_ui(lowM, pm[i][step]);

    r1 = gr + ri;
    fill_sieve(r1, i, step);

    r2 = gr + pm[i][step] - ri;
    fill_sieve(r2, i, step);
}

function fill_sieve(long r_, long i, long step) {
    if (r_ < 0) {
        r_ += pm[i][step];
    }
    while (r_ < (long) Ml2) {
        sieve[r_][i] += 1;
        r_ += pm[i][step];
    }
}

В исходном коде вижу код, который занимается вычислением значения переменной y и никак не затрагивает переменные вне этого фрагмента. Сделаю из него сразу две функции:

function calculate_y(long ri, long i, long step) {
    long y;
    y = ri*ri;
    y -= Npm[i][step];
    y = y/( (long) pm[i][step-1]);
    if (y < 0) { y += pm[i][step]; }
    return y;
}

function correct_y(long y, long i, long step) { 
    long z_rev, z;
    z_rev = (ri<<1) % QR[i];
    if (z_rev == 0) { y = 0; }
    else {
        z = 1;	
        while ((z_rev*z % QR[i]) != 1) { z++; }
        y *= z*pm[i][step-1];
        y = y % pm[i][step];
    }
    return y;
}

С учетом всех проведённых рефакторингов «выделение функции» исходный фрагмент принимает следующий вид:

for (i = 2; i < qr_count; i++) {
		step = 0;
 		ri = r[i];
 		step_sieve(ri, i, step);
		for (step = 1; step < degr[i]; step++) {
			y = calculate_y(ri, i, step);
			y = correct_y(y, i, step);
			ri -= y;
			if (ri < 0) { ri += pm[i][step]; }

			step_sieve(ri, i, step);
		}
	}

Для окончательного блеска сделаю ещё кое-что:

for (long i = 2; i < qr_count; i++) {
 		long ri = r[i];
 		step_sieve(ri, i, 0);
		for (long step = 1; step < degr[i]; step++) {
			ri = decrease_ri(ri, i, step);
			step_sieve(ri, i, step);
		}
	}
	
	function decrease_ri(long ri, long i, long step) {
		long y = calculate_y(ri, i, step);
		y = correct_y(y, i, step);
		ri -= y;
		if (ri < 0) { ri += pm[i][step]; }
		return ri;
	}

Безусловно, суммарно по количеству строк отрефакторенный фрагмент превосходит исходный, однако же для мозга теперь нет необходимости исследовать и понимать весь фрагмент целиком.

Низкая степень понятности операций

Другой проблемный аспект исходного примера кода — низкая степень понятности каждого отдельного его фрагмента из-за невыразительных имён переменных. Чтобы исправить этот аспект, все переменные в примере должны иметь существенно более длинные названия.

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

Теория

  • Длина функций
  • Названия переменных
  • Выделение функции
  • Ребусы в коде
  • Принцип единственного уровня
  • Принцип единственного знания
  • Принцип единственной реализации

Литература

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