📜 ⬆️ ⬇️

Single refactoring analysis

In this tiny post, we will discuss one of the chapters in the book “Principles, Patterns and Agile Development Techniques in C #”, called “Refactoring”. The chapter is completely devoted to refactoring. Using the example of one large method, the author sequentially modifies the code, simultaneously explaining why he makes certain modifications. After each stage, the code is run through the tests.

It is obvious that many examples from books are often synthetic, and are intended only to clarify any thought of the article. Therefore, there are often syntactical and logical errors in the books, and usually this does not impair the perception of the book.

The article does not pursue the goal of discrediting the author, it just seemed interesting to lay out his observations and hear the opinion of the community on this issue.

Let's get started The original method, which the author has changed:
/// <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]; //     , //    } } 


The final version, which ends the article, but the author mentions that theoretically, you can still continue:
 ///<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; } } 


Fluent analysis

In my opinion, in the first example, refactoring is not needed at all, it is enough just to delete unnecessary comments, but then the chapter would not be interesting (smile). In the final version, I am confused only by successive methods that do not accept or return parameters. In my opinion, it is more difficult to understand the logic of such a chain, since there is nothing to cling to.
')
Detailed analysis

So, the first thing we see is that the method is a pure function and has no side effects. Hence its determinism - with the same input parameters, we get the same output parameters. After modification, the author took out local variables from the method to the class, thereby adding state to the class and added side effects to our method. What is bad? This is bad because earlier our method was thread-safe, and after that it became thread-safe. It turns out the author (or we) involuntarily added a potential bug to the method that was refactored, provided it was used in a multithreaded environment.
Secondly, since the variables are still local to the method (s) and do not reflect the state of the class, later on, when adding methods or states, it will be more difficult to figure out whose variables are and why they are needed.

Instead of conclusion

Refactoring is great! But often, we are faced with the problem of eggs and chicken: to understand how the code works, it needs to be refactored, and in order to refactor, you need to understand how it works.

Thank you all for your attention, waiting for your comments.

Source: https://habr.com/ru/post/231325/


All Articles