如何code review代碼累盗？

代碼code review 從大處著眼可以從可讀性寒矿、可維護(hù)性、可擴(kuò)展若债、可復(fù)用符相、可測(cè)試等方面來衡量；落實(shí)到具體細(xì)節(jié)拆座，可以從非功能性和功能性兩個(gè)方面來進(jìn)行衡量主巍。

非功能性

1. 目錄設(shè)置是否合理、模塊劃分是否清晰挪凑、代碼結(jié)構(gòu)是否滿足“高內(nèi)聚孕索，低耦合”特性；
2. 是否遵循經(jīng)典設(shè)計(jì)原則與設(shè)計(jì)思想躏碳，如：SOLID搞旭、DRY、KISS、YAGNI和LOD等肄渗；
3. 設(shè)計(jì)模式是否應(yīng)用合理镇眷，是否過度設(shè)計(jì)；
4. 代碼是否易擴(kuò)展翎嫡，是否滿足開閉原則欠动；
5. 代碼是否可復(fù)用，是否可以復(fù)用已有代碼或類庫惑申，是否存在重復(fù)造輪子具伍；
6. 代碼是否易測(cè)試，UT對(duì)正常和異常邊界情況是否覆蓋全面圈驼；
7. 代碼是否可讀人芽，是否滿足編碼規(guī)范，如：命名是否準(zhǔn)確達(dá)意绩脆、注釋是否恰當(dāng)萤厅、代碼風(fēng)格是否一致、編程是否存在多層嵌套靴迫，復(fù)雜邏輯或多個(gè)入?yún)⑶闆r是否進(jìn)行拆分或封裝等惕味。

功能性

1. 代碼設(shè)計(jì)是否符合功能預(yù)期；
2. 邏輯是否正確矢劲，異常邊界情況是否處置合理赦拘；
3. 日志打印是否得當(dāng)，是否方便排查問題芬沉？
4. 接口是否易用躺同，是否支持CUD場(chǎng)景事務(wù)和冪等操作；
5. 并發(fā)場(chǎng)景下代碼是否線程安全丸逸，是否存在共享變量蹋艺；
6. 性能是否有優(yōu)化空間，如：SQL黄刚、算法是否可以繼續(xù)優(yōu)化捎谨；
7. 是否存在安全漏洞，輸入憔维、輸出校驗(yàn)是否全面合理涛救；

案例

需求

為了方便排查問題，請(qǐng)?jiān)O(shè)計(jì)一個(gè)ID 生成器业扒，每次請(qǐng)求會(huì)將生成的ID 保存在 Servlet 線程的 ThreadLocal 中检吆，每次打印日志的時(shí)候，我們從請(qǐng)求上下文中取出請(qǐng)求 ID程储，跟日志一塊輸出蹭沛。這樣臂寝，同一個(gè)請(qǐng)求的所有日志都包含同樣的請(qǐng)求 ID 信息，我們就可以通過請(qǐng)求 ID 來搜索同一個(gè)請(qǐng)求的所有日志了摊灭。

實(shí)現(xiàn)

@Slf4j
public class IdGenerator {
    /**
     * 獲取請(qǐng)求ID
     *
     * @return
     */
    public static String generate(){
        String id = "";
        try{
            String hostName = InetAddress.getLocalHost().getHostName();
            String[] tokens = hostName.split("\\.");
            if(tokens.length > 0){
                hostName = tokens[tokens.length-1];
            }
            char[] randomChars = new char[8];
            int count = 0;
            Random random = new Random();
            while(count < 8){
                int randomAscii = random.nextInt(122);

                if(randomAscii >= 48 && randomAscii <= 57){
                    randomChars[count] = (char) (('0')+(randomAscii -48));
                    count++;
                }else if(randomAscii >= 65 && randomAscii <= 90) {
                    randomChars[count] = (char) (('A') + (randomAscii - 65));
                    count++;
                }else if(randomAscii >= 97 && randomAscii <= 122){
                    randomChars[count] = (char) (('a')+(randomAscii -97));
                    count++;
                }
            }
            id = String.format("%s-%d-%s",hostName,System.currentTimeMillis(),new String(randomChars));
        }catch (UnknownHostException e){
            log.warn("Failed to get the host name.",e);
        }
        return id;
    }
}

優(yōu)化

非功能分析

功能分析

異常處理

函數(shù)出錯(cuò)返回?cái)?shù)據(jù)類型托猩，我總結(jié)了 4 種情況分別是：錯(cuò)誤碼印蓖、NULL 值、空對(duì)象京腥、異常對(duì)象赦肃。

錯(cuò)誤碼

在沒有異常語法機(jī)制的語言中，常用錯(cuò)誤碼來處理錯(cuò)誤公浪，比如：code >= 0 表示接口調(diào)用成功他宛；code < 0表示接口調(diào)用失敗欠气；同時(shí)code值也可以賦予特殊意義厅各。

NULL值

在多數(shù)編程語言中，我們用 NULL 來表示“不存在”這種語義预柒。對(duì)于查找函數(shù)來說队塘，數(shù)據(jù)不存在并非一種異常情況，是一種正常行為宜鸯，所以返回表示不存在語義的 NULL 值比返回異常更加合理憔古。

空對(duì)象

返回 NULL 值有各種弊端，對(duì)此有一個(gè)比較經(jīng)典的應(yīng)對(duì)策略淋袖，那就是應(yīng)用空對(duì)象設(shè)計(jì)模式鸿市。當(dāng)函數(shù)返回的數(shù)據(jù)是字符串類型或者集合類型的時(shí)候，我們可以用空字符串或空集合替代 NULL 值即碗，來表示不存在的情況焰情。這樣，我們?cè)谑褂煤瘮?shù)的時(shí)候拜姿，就可以不用做 NULL 值判斷烙样。

異常對(duì)象

對(duì)于接口拋出的異常，我們有三種處理方法：直接吞掉蕊肥、直接往上拋出谒获、包裹成新的異常拋出。

適用場(chǎng)景

• 直接吞掉：如果 被調(diào)用方拋出的異常是可以恢復(fù)壁却，的調(diào)用方并不關(guān)心此異常批狱，我們完全可以在 調(diào)用方中拋出的異常吞掉；
• 直接往上拋出：如果被調(diào)用方拋出的異常對(duì)調(diào)用方來說展东，也是可以理解的赔硫、關(guān)心的 ，并且在業(yè)務(wù)概念上有一定的相關(guān)性盐肃，我們可以選擇直接將 被調(diào)用方拋出的異常 re-throw爪膊；
• 包裹成新的異常拋出：如果被調(diào)用方拋出的異常太底層权悟，對(duì)調(diào)用方來說，缺乏背景去理解推盛、且業(yè)務(wù)概念上無關(guān)峦阁，我們可以將它重新包裝成調(diào)用方可以理解的新異常，然后 re-throw耘成。

優(yōu)化計(jì)劃

重構(gòu)代碼的過程需要遵循循序漸進(jìn)榔昔，小步快跑思路。每次改動(dòng)一點(diǎn)點(diǎn)瘪菌，測(cè)試通過之后再進(jìn)行下一部分撒会。針對(duì)本次重構(gòu)計(jì)劃可以分成四部分進(jìn)行，具體如下：

• 第一輪：提高可讀性师妙；

  具體來說分別是：

  1. hostName 變量不應(yīng)該被重復(fù)使用诵肛，尤其當(dāng)這兩次使用時(shí)的含義還不同的時(shí)候；
  2. 將獲取 hostName 的代碼抽離出來默穴，定義為 getLastfieldOfHostName() 函數(shù)曾掂；
  3. 刪除代碼中的魔法數(shù)；
  4. generate() 函數(shù)中的三個(gè) if 邏輯重復(fù)了壁顶，且實(shí)現(xiàn)過于復(fù)雜，我們要對(duì)其進(jìn)行簡化溜歪；
  5. 對(duì) IdGenerator 類重命名若专，并且抽象出對(duì)應(yīng)的接口。

• 第二輪：提高可測(cè)試性蝴猪；

  具體來說分別是：

  1. generate() 函數(shù)定義為靜態(tài)函數(shù)调衰，會(huì)影響使用該函數(shù)的代碼的可測(cè)試性，需要改為非靜態(tài)函數(shù)自阱；
  2. generate() 函數(shù)的代碼實(shí)現(xiàn)依賴運(yùn)行環(huán)境（本機(jī)名）嚎莉、時(shí)間函數(shù)、隨機(jī)函數(shù)測(cè)試性不好需要進(jìn)行對(duì)相應(yīng)邏輯進(jìn)行拆分和封裝沛豌，具體來下：1趋箩、從 getLastfieldOfHostName() 函數(shù)中，將邏輯比較復(fù)雜的那部分代碼剝離出來加派，定義為 getLastSubstrSplittedByDot() 函數(shù)叫确；2、將 generateRandomAlphameric() 和 getLastSubstrSplittedByDot() 這兩個(gè)函數(shù)的訪問權(quán)限設(shè)置為 protected芍锦。這樣做的目的是竹勉，可以直接在單元測(cè)試中通過對(duì)象來調(diào)用兩個(gè)函數(shù)進(jìn)行測(cè)試。3娄琉、給 generateRandomAlphameric() 和 getLastSubstrSplittedByDot() 兩個(gè)函數(shù)添加@VisibleForTesting次乓。告訴其他人說吓歇，這兩個(gè)函數(shù)本該是 private 訪問權(quán)限的，之所以提升訪問權(quán)限到 protected票腰，只是為了測(cè)試城看，只能用于單元測(cè)試中。
  3. 針對(duì)異常和特殊值進(jìn)行適當(dāng)處理丧慈，拋出調(diào)用方理解的異常析命。

• 第三輪：編寫完成UT，寫單元測(cè)試的時(shí)候逃默，測(cè)試對(duì)象是函數(shù)定義的功能鹃愤，而非具體的實(shí)現(xiàn)邏輯；

  針對(duì)generate()函數(shù)可以有三個(gè)不同功能的定義完域；

  • 如果我們把 generate() 函數(shù)的功能定義為：“生成一個(gè)隨機(jī)唯一 ID”软吐，那我們只要測(cè)試多次調(diào)用 generate() 函數(shù)生成的 ID 是否唯一即可。
  • 如果我們把 generate() 函數(shù)的功能定義為：“生成一個(gè)只包含數(shù)字吟税、大小寫字母和中劃線的唯一 ID”凹耙，那我們不僅要測(cè)試 ID 的唯一性，還要測(cè)試生成的 ID 是否只包含數(shù)字肠仪、大小寫字母和中劃線肖抱。
  • 如果我們把 generate() 函數(shù)的功能定義為：“生成唯一 ID，格式為：{主機(jī)名 substr}-{時(shí)間戳}-{8 位隨機(jī)數(shù)}异旧。在主機(jī)名獲取失敗時(shí)意述，返回：null-{時(shí)間戳}-{8 位隨機(jī)數(shù)}”，那我們不僅要測(cè)試 ID 的唯一性吮蛹，還要測(cè)試生成的 ID 是否完全符合格式要求荤崇。

  總結(jié)：UT如何寫，關(guān)鍵看你如何定義函數(shù)潮针。

• 第四輪：添加完整注釋术荤，注釋主要就是寫清楚：做什么、為什么每篷、怎么做瓣戚、怎么用，對(duì)一些邊界條件雳攘、特殊情況進(jìn)行說明带兜，以及對(duì)函數(shù)輸入、輸出吨灭、異常進(jìn)行說明刚照。；

優(yōu)化結(jié)果

/**
 * IdGenerator接口 重構(gòu)一 提高代碼可擴(kuò)展性
 */
public interface IdGenerator {
    public String generate() throws IdGenerationFailureException;
}
/**
 * LogTraceIdGenerator接口 重構(gòu)一 提高代碼可擴(kuò)展性
 */
public interface LogTraceIdGenerator extends IdGenerator{
}
/**
 * 用于生成隨機(jī) ID 的 Id 生成器喧兄。//重構(gòu)四：添加注釋
 *
 *
 * 此類生成的 ID 不是絕對(duì)唯一的无畔，
 * 但重復(fù)的可能性非常低啊楚。
 */
@Slf4j
public class RandomIdGenerator implements LogTraceIdGenerator{//重構(gòu)一 提高代碼可擴(kuò)展性
    /**
     * 生成隨機(jī) ID。只有在極端情況下浑彰，才能生成重復(fù)ID恭理。
     *
     * @return隨機(jī) ID
     */
    @Override
    public String generate() throws IdGenerationFailureException {//重構(gòu)一
        String substrOfHostName = null;
        try {
            substrOfHostName = getLastfieldOfHostName();
        } catch (UnknownHostException e) { //重構(gòu)二：封裝新異常繼續(xù)拋出
           throw new IdGenerationFailureException("host name is empty.");
        }
        //if(substrOfHostName == null || substrOfHostName.isEmpty()){//重構(gòu)二：異常處理
        //    throw new IdGenerationFailureException("host name is empty.");
        //}
        long currentTimeMillis = System.currentTimeMillis();

        String randomString = generateRandomAlphameric(8);//重構(gòu)二 提供代碼可測(cè)試性
        String id = String.format("%s-%d-%s", substrOfHostName, currentTimeMillis, randomString);
        return id;
    }

    /**
     * 獲取本地主機(jī)名和
     * 提取由分隔符 '.' 拆分的名稱字符串的最后一個(gè)字段。
     *
     * @return 主機(jī)名的最后一個(gè)字段郭变。如果未獲取主機(jī)名颜价，則返回 null。
     */
    private String getLastfieldOfHostName() throws UnknownHostException {// 重構(gòu)一
        String substrOfHostName = null;
        //try {
        String hostName = InetAddress.getLocalHost().getHostName();

        //重構(gòu)二 NULL判斷
        if (hostName == null || hostName.isEmpty()) {
            throw new UnknownHostException();
        }
        //重構(gòu)二 邏輯拆分
        substrOfHostName = getLastSubstrSplittedByDot(hostName);
        //String[] tokens = hostName.split("\\.");
        //substrOfHostName = tokens[tokens.length -1];

        //}catch (Exception e){////重構(gòu)二：異常處理诉濒，直接拋出
        //    log.warn("Failed to get the host name.",e);
        //}
        return substrOfHostName;

    }

    /**
     * 獲取 {@hostName} 的最后一個(gè)字段周伦，該字段由 delemiter '.' 拆分。
     *
     * @param hostName 主機(jī)名不應(yīng)為空
     * @return {@hostName} 的最后一個(gè)字段未荒。如果 {@hostName} 為空字符串专挪，則返回空字符串。
     */
    @VisibleForTesting //重構(gòu)二
    protected String getLastSubstrSplittedByDot(String hostName) {//重構(gòu)二 提供代碼可測(cè)試性
        //重構(gòu)二：NULL 判斷片排，如果傳入NULL 則拋運(yùn)行異常
        if(hostName == null || hostName.isEmpty()){
            throw new IllegalArgumentException("host name is empty");
        }
        String[] tokens = hostName.split("\\.");
        String substrOfHostName = tokens[tokens.length -1];
        return substrOfHostName;
    }

    /**
     * 生成隨機(jī)字符串寨腔，其中
     * 僅包含數(shù)字、大寫字母和小寫字母率寡。
     *
     * @param length 長度應(yīng)不小于0
     * @return 隨機(jī)字符串 如果 {@length} 為 0 則返回空字符串
     *
     */
    @VisibleForTesting //重構(gòu)二
    protected String generateRandomAlphameric(int length) {//重構(gòu)一 復(fù)雜邏輯拆分 //重構(gòu)二 為了提高測(cè)試性迫卢，將private 改為 protected
        if(length < 0){ //重構(gòu)二 邊界處理
            throw new IllegalArgumentException("parameter length is illegal.");
        }

        char[] randomChars = new char[length];
        int count = 0;
        Random random = new Random();
        while (count < length){
            int maxAscii = 'z';
            int radomAscii = random.nextInt(maxAscii);
            //重構(gòu)一 提高代碼可讀性
            boolean isDigit = radomAscii >= '0' && radomAscii < '9';
            boolean isLowercase = radomAscii >= 'a' && radomAscii < 'z';
            boolean isUppercase = radomAscii >= 'A' && radomAscii < 'Z';
            if(isDigit || isLowercase || isUppercase){
                randomChars[count] = (char) radomAscii;
                count++;
            }
        }
        return new String(randomChars);
    }
}

/**
 * RandomIdGeneratorTest類 重構(gòu)三：編寫UT
 */
public class RandomIdGeneratorTest {

    @Test
    public void testGetLastSubstrSplittedByDot(){
        RandomIdGenerator generator = new RandomIdGenerator();
        String actualSubstr = generator.getLastSubstrSplittedByDot("field1.field2.field3");
        Assert.assertEquals("field3",actualSubstr);

        actualSubstr = generator.getLastSubstrSplittedByDot("field1");
        Assert.assertEquals("field1",actualSubstr);

        actualSubstr = generator.getLastSubstrSplittedByDot("field1#field2#field3");
        Assert.assertEquals("field1#field2#field3",actualSubstr);
    }

    @Test
    public void testGetLastSubstrSplittedByDot_nullOrEmpty(){
        RandomIdGenerator generator = new RandomIdGenerator();
        String actualSubstr = generator.getLastSubstrSplittedByDot("");
        Assert.assertEquals("",actualSubstr);

        actualSubstr = generator.getLastSubstrSplittedByDot(null);
        Assert.assertNull(actualSubstr);
    }

    @Test
    public void testGenerateRandomAlphameric(){
        RandomIdGenerator generator = new RandomIdGenerator();
        String randomString = generator.generateRandomAlphameric(8);
        Assert.assertNotNull(randomString);
        Assert.assertEquals(8,randomString.length());
        for (char s : randomString.toCharArray()) {
            Assert.assertTrue((s >= '0' && s < '9') || (s >= 'a' && s < 'z') || (s >= 'A' && s < 'Z'));
        }
    }

    @Test
    public void testGenerateRandomAlphameric_lengthEqualsOrLessThanZero(){
        RandomIdGenerator generator = new RandomIdGenerator();
        String randomString = generator.generateRandomAlphameric(0);
        Assert.assertEquals("",randomString);

        randomString = generator.generateRandomAlphameric(-1);
        Assert.assertNull(randomString);
    }

    /**
     * generate() 函數(shù)的功能定義為：“生成唯一 ID，格式為：{主機(jī)名 substr}-{時(shí)間戳}-{8 位隨機(jī)數(shù)}
     */
    @Test
    public void testGenerate(){
        RandomIdGenerator generator = new RandomIdGenerator();
        String id1 = generator.generate();
        Assert.assertNotNull(id1);
        Assert.assertTrue(id1.length() > 0);
        for (char c: id1.toCharArray()) {
            Assert.assertTrue((c == '-')||(c >= '0' && c < '9') || (c >= 'a' && c < 'z') || (c >= 'A' && c < 'Z'));

        }
        String id2 = generator.generate();
        Assert.assertNotNull(id1);
        Assert.assertTrue(id1.length() > 0);
        for (char c: id2.toCharArray()) {
            Assert.assertTrue((c == '-')||(c >= '0' && c < '9') || (c >= 'a' && c < 'z') || (c >= 'A' && c < 'Z'));

        }

        Assert.assertNotEquals(id1,id2);
    }

    /**
     * generate() 函數(shù)在主機(jī)名獲取失敗時(shí)冶共，返回：null-{時(shí)間戳}-{8 位隨機(jī)數(shù)}”,測(cè)試生成的 ID 是否完全符合格式要求靖避。
     */
    @Test
    public void testGenerate_withoutSubHostName(){
        RandomIdGenerator generator = new RandomIdGenerator();
        generator.getLastSubstrSplittedByDot(null);

        String id = generator.generate();
        Assert.assertNotNull(id);
        Assert.assertTrue(id.length() > 0);
        String[] split = id.split("-");
        Assert.assertTrue(split[0] == null);
        Assert.assertTrue(System.currentTimeMillis() == Long.valueOf(split[1]));
        for (char c: split[2].toCharArray()) {
            Assert.assertTrue((c >= '0' && c < '9') || (c >= 'a' && c < 'z') || (c >= 'A' && c < 'Z'));

        }
    }
}