Очевидно, что многие примеры из книг, часто являются синтетическими, и предназначены только для пояснения какой-либо мысли статьи. По этому часто в книгах присутствуют как синтаксические так и логические ошибки, и обычно, это ни как не ухудшает восприятие книги.
Статья не преследует цели дискредитации автора, просто показалось интересным выложить свои наблюдения и услышать мнение сообщества по этому поводу.
/// <remark> /// Этот класс генерирует простые числа, не превышающие заданного /// пользователем порога. В качестве алгоритма используется решето /// Эратосфена. /// /// Эратосфен Киренский, 276 г. до н. э., Кирена, Ливия -- /// 194 г. до н. э., Александрия. Впервые измерил окружность Земли. /// Известен также работами по составлению календаря с високосными /// годами. Работал в Александрийской библиотеке. /// /// Сам алгоритм прост. Пусть дан массив целых чисел, начинающийся /// с 2. Вычеркиваем все кратные 2. Находим первое невычеркнутое /// число и вычеркиваем все его кратные. Повторяем, пока не /// дойдем до квадратного корня из максимального значения. /// /// Написал Роберт К. Мартин 9.12.1999 на языке Java /// Перевел на C# Мика Мартин 12.01.2005. ///</remark> using System; /// <summary> /// автор: Роберт К. Мартин /// </summary> public class GeneratePrimes { ///<summary> /// Генерирует массив простых чисел. ///</summary> /// /// <param name=”maxValue”>Верхний порог.</param> public static int[] GeneratePrimeNumbers(int maxValue) { if (maxValue >= 2) // единственный допустимый случай { // объявления int s = maxValue + 1; // размер массива bool[] f = new bool[s]; int i; // инициализировать элементы массива значением true. for (i = 0; i < s; i++) f[i] = true; // исключить заведомо не простые числа f[0] = f[1] = false; // решето int j; for (i = 2; i < Math.Sqrt(s) + 1; i++) { if (f[i]) // если i не вычеркнуто, вычеркнуть его кратные. { for (j = 2 * i; j < s; j += i) f[j] = false; // кратное – не простое число } } // сколько оказалось простых чисел? int count = 0; for (i = 0; i < s; i++) { if (f[i]) count++; // увеличить счетчик } int[] primes = new int[count]; // поместить простые числа в результирующий массив for (i = 0, j = 0; i < s; i++) { if (f[i]) // если простое primes[j++] = i; } return primes; // вернуть простые числа } else // maxValue < 2 return new int[0]; // если входные данные не корректны, // возвращаем пустой массив } }
///<remark> /// Этот класс генерирует простые числа, не превышающие заданного /// пользователем порога. В качестве алгоритма используется решето /// Эратосфена. /// Пусть дан массив целых чисел, начинающийся с 2: /// Находим первое невычеркнутое число и вычеркиваем все его /// кратные. Повторяем, пока в массиве не останется кратных. ///</remark> using System; public class PrimeGenerator { private static bool[] crossedOut; private static int[] result; public static int[] GeneratePrimeNumbers(int maxValue) { if (maxValue < 2) return new int[0]; else { UncrossIntegersUpTo(maxValue); CrossOutMultiples(); PutUncrossedIntegersIntoResult(); return result; } } private static void UncrossIntegersUpTo(int maxValue) { crossedOut = new bool[maxValue + 1]; for (int i = 2; i < crossedOut.Length; i++) crossedOut[i] = false; } private static void PutUncrossedIntegersIntoResult() { result = new int[NumberOfUncrossedIntegers()]; for (int j = 0, i = 2; i < crossedOut.Length; i++) { if (NotCrossed(i)) result[j++] = i; } } private static int NumberOfUncrossedIntegers() { int count = 0; for (int i = 2; i < crossedOut.Length; i++) { if (NotCrossed(i)) count++; // увеличить счетчик } return count; } private static void CrossOutMultiples() { int limit = DetermineIterationLimit(); for (int i = 2; i <= limit; i++) { if (NotCrossed(i)) CrossOutputMultiplesOf(i); } } private static int DetermineIterationLimit() { // У каждого составного числа в этом массиве есть простой // множитель, меньший или равный квадратному корню из размера // массива, поэтому необязательно вычеркивать кратные, большие // корня. double iterationLimit = Math.Sqrt(crossedOut.Length); return (int)iterationLimit; } private static void CrossOutputMultiplesOf(int i) { for (int multiple = 2 * i; multiple < crossedOut.Length; multiple += i) crossedOut[multiple] = true; } private static bool NotCrossed(int i) { return crossedOut[i] == false; } }
Беглый анализ
На мой взгляд, в первом примере вообще рефакторинг не нужен, достаточно просто удалить лишние комментарии, но тогда глава была бы не интересная (смайл). В конечном варианте меня смущает только подряд идущие методы, которые не принимают и не возвращают параметры. По моему ощущению, понять логику такой цепочки сложнее, так как не за что зацепиться.
Подробный анализ
Итак, первое что мы видим, то что метод является чистой функцией и не имеет побочных эффектов. Отсюда вытекает его детерминированность — при одних и тех же входных параметрах мы получаем одни и те же выходные параметры. После модификации, автор вынес локальные переменные из метода в класс, тем самым добавив состояние классу и добавил побочные эффекты нашему методу. Чем это плохо? Плохо это тем, что раньше наш метод был потокобезопасным, а после этого стал потокоопасным. Получается автор (или мы), невольно добавили потенциальный баг в метод, который рефакторили, при условии использования в многопоточной среде.
Во вторых, так как переменные все же являются локальными для метода (методов) и не отражают состояние класса, то в дальнейшем, при добавлении методов или состояния, будет сложнее разбираться, чьи же это переменные и зачем они нужны.
Вместо заключения
Рефакторинг это прекрасно! Но часто, мы сталкиваемся с проблемой яйца и курицы: чтобы понять как работает код, его необходимо отрефакторить, а чтобы отрефакторить, нужно понимать как он работает.
Всем спасибо за внимание, жду ваших комментариев.
ссылка на оригинал статьи http://habrahabr.ru/post/231325/
Добавить комментарий