重构代码初体验-1

一个矫正代码习惯的练习记录

原代码如下：

package literatePrimes;
public class PrintPrimes {
  public static void main(String[] args) {
    final int M = 1000;
    final int RR = 50;
    final int CC = 4;
    final int WW = 10;
    final int ORDMAX = 30;
    int P[] = new int[M + 1];
    int PAGENUMBER;
    int PAGEOFFSET;
    int ROWOFFSET;
    int C;int J;
    int K;
    boolean JPRIME;
    int ORD;
    int SQUARE;
    int N;
    int MULT[] = new int[ORDMAX + 1];
    J = 1;
    K = 1;
    P[1] = 2;
    ORD = 2;
    SQUARE = 9;
    while (K < M) {
      do {
        J = J + 2;
        if (J == SQUARE) {
          ORD = ORD + 1;
          SQUARE = P[ORD] * P[ORD];
          MULT[ORD - 1] = J;
        }
        N = 2;
        JPRIME = true;
        while (N < ORD && JPRIME) {
          while (MULT[N] < J)
            MULT[N] = MULT[N] + P[N] + P[N];
          if (MULT[N] == J)
            JPRIME = false;
          N = N + 1;
        }
      } while (!JPRIME);
      K = K + 1;
      P[K] = J;
    }
    {
      PAGENUMBER = 1;
      PAGEOFFSET = 1;
      while (PAGEOFFSET <= M) {
        System.out.println("The First " + M + 
                             " Prime Numbers --- Page " + PAGENUMBER);
        System.out.println("");
        for (ROWOFFSET = PAGEOFFSET; ROWOFFSET < PAGEOFFSET + RR; ROWOFFSET++){
          for (C = 0; C < CC;C++)
            if (ROWOFFSET + C * RR <= M)
              System.out.format("%10d", P[ROWOFFSET + C * RR]);
          System.out.println("");
        }
        System.out.println("\f");
        PAGENUMBER = PAGENUMBER + 1;
        PAGEOFFSET = PAGEOFFSET + RR * CC;
      }
    }
  }
}

粗读下来就能发现的问题：

1. 函数太长了，而且杂糅了数据生成和数据打印到页面上，两个模块的代码
2. 变量命名太简单了，看不出实际的含义
3. 数据生成部分的代码逻辑过于复杂，看不懂是怎么生成的数据

先从更容易看懂的数据打印部分开始重构：

PrimePrinter-refactor1

public class PrimePagePrinter {
    int pageNumber = 1; //PAGENUMBER
    int pageOffset = 1; //PAGEOFFSET
    int rowLength = 50; //RR
    int colLength = 4;//CC
    int M = 1000;
    final int WW = 10;
    final int ORDMAX = 30;
    int P[];

    public PrimePagePrinter(int rowLength, int colLength, int[] P) {
        this.rowPerPage = rowLength;
        this.colLength = colLength;
        this.P = P;
        this.M = P.length - 1;
    }

    public print() {
        while (this.pageOffset <= this.M) {
            this.printHeader();
            this.PrintPrimes();
            System.out.println("\f");
            this.pageNumber += 1;
            this.pageOffset += this.rowPerPage * this.colLength;
        }
    }

    public printHeader() {
        System.out.println("The First " + M + 
                             " Prime Numbers --- Page " + this.pageNumber);
        System.out.println("");
    }

    public printPrimes() {
        for (ROWOFFSET = this.pageOffset; ROWOFFSET < this.pageOffset + this.rowPerPage; ROWOFFSET++){
            for (C = 0; C < this.colLength;C++) 
                if (ROWOFFSET + C * this.rowPerPage <= this.M) 
                    System.out.format("%10d", this.P[ROWOFFSET + C * this.rowPerPage]);
          System.out.println("");
        }
    }

这一步重构主要做了：

1. 修改了部分命名，让含义更清晰
2. 对打印过程进行了简单的分割，分成了 printHeader ，printPrimes

但是：

1. 变量M，P的含义从变量名上看仍然不清晰；
2. ROWOFFSET + C * this.rowPerPage 的含义是仍不清晰，从代码里仍不能直观看出为什么要从P中以该变量值作为下标来读取数据

更进一步的问题是，PrimePrinter的代码，是否与Prime本身强关联？是否意味着不能打印其他类型的数据？

这需要看懂打印的逻辑，才能更进一步的抽象。

• ROWOFFSET + C * this.rowPerPage 提示 printPrime函数的目标是在一页的范围内，找到需要输出的值；
• this.pageOffset += this.rowPerPage * this.colLength; 提示 pageOffset递增的单位实际上是每一页具有的所有字符；
• print的while循环实际上是在对页做递增；
• 无论是页递增还是 页面内输出数据的下标递增，都不能超过提供的数据的数组的长度，可以推测实际上printer的含义是把提供的数据，按照某种页面的样式（指定行列数）打印。

因此，对第一轮更改的代码，可以改进的地方如标注：

public class PrimePagePrinter {
    int pageNumber = 1; //PAGENUMBER
    int pageOffset = 1; //PAGEOFFSET
    int rowLength = 50; //RR
    int colLength = 4;//CC
    int M = 1000;
    final int WW = 10;
    final int ORDMAX = 30;
    int P[];

    public PrimePagePrinter(int rowLength, int colLength, int[] P) {
        this.rowPerPage = rowLength;
        this.colLength = colLength;
        // P是外界传入的数据数组；M是下标所能有的最大值
        this.P = P;
        this.M = P.length - 1;
    }

    public print() {
        while (this.pageOffset <= this.M) {
            this.printHeader();
            this.PrintPrimes();
            System.out.println("\f");
            this.pageNumber += 1;
            this.pageOffset += this.rowPerPage * this.colLength;
        }
    }

    public printHeader() {
        System.out.println("The First " + M + 
                             " Prime Numbers --- Page " + this.pageNumber);
        System.out.println("");
    }

    //实际上是打印每一页 不是打印Primes这么抽象的功能
    public printPrimes() {
        //Rowoffset遍历了这一页的所有行，类似二元坐标系里的x值
        for (ROWOFFSET = this.pageOffset; ROWOFFSET < this.pageOffset + this.rowPerPage; ROWOFFSET++){
            // C 是列坐标
            for (C = 0; C < this.colLength;C++) 
                // ROWOFFSET + C * this.rowPerPage 是（x，y）在一维上的展开，不能超过数据下标最大值
                if (ROWOFFSET + C * this.rowPerPage  <= this.M) 
                    //可见打印可以进一步分割成打印页和打印行
                    System.out.format("%10d", this.P[ROWOFFSET + C * this.rowPerPage]);
          System.out.println("");
        }
    }
 

除了函数的分割，作为一个类，其中的变量是否需要作为类的变量也是需要考量的点

import java.io.PrintStream;
//与Prime无强关联，本质是给定行列打印一页
public class RowColumnPagePrinter { 
    // private int pageNumber = 1; //PAGENUMBER
    // private int pageOffset = 1; //PAGEOFFSET
    // pageNumber pageOffset的生命周期与类并不一致，
    // 每次打印都会重新set，所以从类中移除

    private int rowPerPage = 50; //RR clearer than rowLength
    private int colPerPage = 4; //CC clearer than colLength
    private int numbersPerPage; // in printPrimes(), rowPerPage * colPerPage numbers will be printed 
    // final int WW = 10;
    // final int ORDMAX = 30; not used in print
    // why name does not contain 'prime': the class defines the context of the code
    
    // P => int data[];
    // 但是可以作为外界参数传入，同一个printer可以在生命周期内打印多个不同的数组，因此不需要作为类的成员变量

    // printHeader中的常量，可以与prime无关，即是一个与printer具体实现无关，可以被调用者指定的值
    private String pageHeader;

    // 提供了拓展性：可以使用System.out之外的PrintStream
    private PrintStream printStream;

    public RowColumnPagePrinter(int rowPerPage, int colPerPage, String pageHeader) {
        this.rowPerPage = rowPerPage;
        this.colPerPage = colPerPage;
        this.numbersPerPage = this.colPerPage * this.rowLength;
        this.pageHeader = pageHeader;
        printStream = System.out;
    }

    // has default, also has way to change
    public SetPrintStream(PrintStream printStream) {
        this.printStream = printStream;
    }

    public print(int[] data) {
        // <= M & M = data.length-1 => < data.length
        // pageOffset => firstIndexOnPage; 是每一行的开头，
        // 类似内存的定位方式，再加上偏移量才是数据的最终位置，所以pageOffset的隐藏含义是每页的开头
        int pageNumber = 1;
        for (int firstIndexOnPage = 0;
        firstIndexOnPage < data.length;
        firstIndexOnPage += numbersPerPage) {
            // 计算出每页最后一个坐标，不应当超过数组最大下标
            int lastIndexOnPage = Math.min(data.length-1, firstIndexOnPage + numbersPerPage-1);
            printHeader(pageHeader,pageNumber);
            printPage(firstIndexOnPage, lastIndexOnPage, data);
            // really for page to end
            printStream.println("\f");
            pageNumber += 1;
        }
        // while (this.pageOffset < data.length) {
        //     this.printHeader();
        //     this.printPage();
        //     this.pageOffset += this.numbersPerPage;
        //     this.pageNumber += 1;
        // }
    }

    // change to private because it is for print
    
    private printHeader(String pageHeader, int pageNumber) {
        // System.out.println("The First " + this.data.length +1 + 
        //                      " Prime Numbers --- Page " + this.pageNumber);
        // System.out.println("");
        printStream.println(pageHeader + " --- Page " + pageNumber);
        printStream.println("");
    }
    
    private printPage(int firstIndexOnPage, int lastIndexOnPage,int[] data) {
        // for (int firstIndex = this.pageOffset; firstIndex<this.pageOffset + this.rowLength; firstIndex += 1) {
        //     // this lastIndex is for all the page so should be seperated from firstIndex
        //     int lastIndex = Math.min(data.length-1, firstIndex + ((this.colPerPage -1) * this.rowLength));
        //     // no, actually (this.colPerPage -1) * this.rowLength is first index of last row
        //     this.printPrimes(firstIndex, lastIndex, data); 此处还是没有把页和行分开
        //     System.out.println("\f");
        // }
        // 计算出最后一行的开头坐标，作为每一行开头坐标的 最大值
        int firstIndexOfLastRowOnPage = firstIndexOnPage + rowPerPage - 1;
        for (int firstIndexOnRow = firstIndexOnPage; 
        firstIndexOnRow <= firstIndexOfLastRowOnPage;
        firstIndexOnRow+=1) {
            printRow(firstIndexOnRow, lastIndexOnPage, data);
            printStream.println("");
        }
    }

    // last index should be smaller than data length
    // from printPrims => printRow
    private printRow(int firstIndexOnRow, int lastIndexOnPage, int[] data) {
        // 首尾已经被确定，打印的就是指定的 原代码中的 ROWOFFSET + C * this.rowPerPage
        for (int index = firstIndex; index<=lastIndex; index += this.rowLength)
            System.out.format("%10d", this.data[index]);
        System.out.println("");
    }
}

printRow函数的另一种实现：

private printRow(int firstIndexOnRow, int lastIndexOnPage, int[] data) {
    // 含义更清晰 C => column 
    for (int column = 0; column < colPerPage; column +=1) {
        //firstIndexOnRow => x, column => y
        // index就是原代码里 ROWOFFSET + C * this.rowPerPage
        int index = firstIndexOnRow + column * rowPerPage;
        if (index < lastIndexOnPage)
            printStream.format("%10d", this.data[index]);
        // or if index > lastIndexOnPage return
    }
}

第二版重构做了：

1. 明确的类的作用：打印，与打印的数据无关，在命名上体现了
2. 移除了和类生命周期不一致的成员变量
3. 把打印进一步分成了打印页和行，通过确认firstIndex，lastIndex来划定边界。

改完打印部分，意味着我们有一个Printer，只需要指定pageHeader，rowPerPage，colPerPage，data，就可以按照原代码的格式打印出任意给定的数据（包括Primes）。下一篇就来改真正的x山，实在是看不懂的生成Prime的部分代码。