記一次Lombok的Setter過載方法造成的事故及思考

Jayden.李發表於2019-03-31

前言

Lombok是很多Java開發者會用到的一個非常方便的Java庫。在lombok的幫助下,開發者將更加集中於業務邏輯的開發,而不受重複的,無聊的,非業務邏輯程式碼書寫的影響。一個比較明顯的例子就是我們不需要再手動為每一個類成員變數寫Setter和Getter方法。

問題

Lombok在為我們帶來便利的同時,不僅解放了開發者的雙手,一定程度上也讓開發者忘記了思考到底Lombok為我們自動生成了什麼程式碼。如果不深究Lombok的自動程式碼生成原理,則會讓我們忽略掉很多Java基礎知識。

先看下面一段程式碼,請問request為”/hello”的時,我們所得到的Response的body是什麼?

@Getter
@Setter
abstract class A {
	private boolean success;
}

@Setter
@Getter
class B extends  A {
	public Boolean success;

}


@Controller
@RequestMapping("/hello")
public class Hello {

	@GetMapping
	@ResponseBody
	public B returnB() {
		B b = new B();
		b.setSuccess(true);
		return b;
	}
}

複製程式碼

答案是:

body = {success: null}
複製程式碼

原因

其實原理很簡單,但是很容易被我們忽略掉。我問過幾個老Java開發,他們雖然知道其中的原理,但是在一開始問他們為什麼success的值為null的時候,都沒有人能立刻想到原因。

其實造成success的值為null的原因不是Lombok, Lombok生成的Setter過載了父類的方法。

首先,我們想象一下,如果我們不使用Lombok的Setter和Getter的話,我們自己也許會像下面手動寫Getter和Setter。

class A {
  public boolean success;

  public void setSuccess(boolean success) {
  	this.success = success;
  }
  public boolean getSuccess() {
    return this.success;
  }
}

class B extends A {
  public Boolean success;

  public void setSuccess(Boolean success) {
  	this.success = success;
  }
  @Override
  public boolean getSuccess() {
    return this.success;
  }
}

複製程式碼

上面的程式碼, B的setSuccess過載了A的setSuccess,同時,B的getSuccess重寫了A.getSuccess.

由於B的setSuccess過載了A的setSuccess.所以B的物件裡面就會有兩個方法,Signature分別是:

void setSuccess(boolean success), 
void setSuccess(Boolean success)
複製程式碼

所以當我們使用b.setSuccess(true)時, 這裡不是設定的b.success(Boolean)的值,而是設定的b.A.success(boolean)的值,同時,b.A.success是一個隱藏值,所以對於外部來說,根本看不見b.A.success的值。如下圖所示。

記一次Lombok的Setter過載方法造成的事故及思考

而此時b.success的值則為初始值null。要想設定b.success的值的話,則需要使用b.setSuccess(Boolean.True)

如果這個setSuccess是自己去寫的話,我們可以能注意到子類和父類的入參型別的變化,而加以注意,不至於造成選錯setSuccess方法,而導致功能行為與預想的不一致。

事故模擬

在我們的Legacy程式碼裡面,有一個父類,這個父類裡面有一個成員變數和一個方法。如下:

@Setter
@Getter
class A {
  private boolean success;

  public void doSomethingAndSetSuccess() {
		// dosomething
     this.setSuccess(true);
  }
}
複製程式碼

同時,在Legeacy程式碼裡面,還有一個子類繼承了這個父類, 如下:

class B extends A {
  // something not important
}
複製程式碼

然後,在這個在這個Legacy系統裡面,有對B的物件呼叫。如下:

B b = new B();
b.doSomethingAndSetSuccess();

// something here
return b;  (通過@ResponseBody)
複製程式碼

這樣的結構之前一直執行得很好,所返回的json結果一直都是我們想要的結果,比如{success: true(or false)}。直到有一天,有一個開發沒有注意到他們之間的關係。在類B上加入瞭如下程式碼:

@Getter
@Setter
class B extends A {
  public Boolean success = null;

  // the last code same as before.
}
複製程式碼

Lombok的Setter,過載了A類裡面的setSuccess(boolean success),進而,在不修改A.doSomethingAndSetSuccess裡面的setSuccess方法的話,則永遠沒有地方呼叫B的setSuccess(Boolean success)。也就是說B的success永遠是初始值null.

事故反思與預防

  1. 到底類的成員變數使用封裝的資料型別還是使用原始資料型別?

    從上面的事故中,我們會容易得出一個結論,就是在子類中新新增的成員變數資料型別和父類不同造成了事故,亦即開發規範不同引起了意外。

    那麼,為了團隊以後不再出現類似的類成員變數型別不一致的情況,我們到底應該是選擇像父類那樣,使用原始變數型別,還是使用包裝型別呢?

    先來看看阿里巴巴開發手冊(下手冊)上是怎麼說的吧。

記一次Lombok的Setter過載方法造成的事故及思考

手冊上舉了兩個例子來論證類屬性強制使用包裝資料型別。總結其論點就是 1. 包裝資料型別的RPC返回值可以是null,代示其“不存在”。 2. 因為類屬性是引用型別,就算直接賦值null,不會有NPE風險。

記一次Lombok的Setter過載方法造成的事故及思考

這兩個論據很好理解,我也表示贊同。有一點需要補充的是,我認為比起返回原始資料的包裝資料型別(Integer,Boolean,etc),返回一個自定義的資料型別會更加有語意。

假設你有一個RPC類A,程式碼如下:

// 返回原始資料型別
class A {
  public int a;

  public int getA(){
    // do some logic.
    return this.a;
  }
}
// code 1
複製程式碼
class A {
	public Integer a;

	public Integer getA(){
		// do some logic
		return this.a;
	}
}
//code2
複製程式碼

正如手冊所說,code 2比起code 1,能夠多返回一個null。因此也能告訴呼叫者你想呼叫的值根本不存在。

但是,我認為作為一個開發,當你知道當前所要返回的值將會是 -x,0,x和不存在(姑且叫null),雖然返回Integer是一個不錯的解決方案,但我覺得從語意的角度上來說,還是不夠好。畢竟null不是一個Integer,它只是一個空的Integer物件的引用。

所以我覺得在這種情況,最有語意的做法是自定義一個新的資料結構。

@Setter
@Getter
class SomeResult {
	// 使用這種編碼方式,可以不用每一個POJO類屬性都為包裝資料型別。只需要保證RPC類屬性為自定義資料型別即可。
	private boolean exist = true;
	private int value = 0;
}

// 可以再把SomeResult再抽象,以減少重複程式碼。這裡不再列出。

class A {
  public SomeResult someResult;

  public SomeResult returnSomeResult() {
		// after doing some logic, return result
		// 我覺得這裡,作為程式設計師,不應該主動返回null. 返回null一時爽,語意不清,造成後續Debug難度增加。
		// 關於返回null的部落格,詳細請移步  [https://juejin.im/post/5ba88342e51d450e6475f7cd](https://juejin.im/post/5ba88342e51d450e6475f7cd) 
		if(someResult exists) {
			return someResult;
		} else {
			someResult.setExist(fasle);
			return someResult;
		}
  }

}

class B {
  public void someMethod() {

		SomeResult someResult = getSomeResultBySomeService();
		
		// 作為呼叫者,確實有責任和義務確認RPC返回值是否為null。null在語意上來說這是一個Exception.不應該參與業務邏輯的意外。
		// 使用這種自定義資料結構,明確地表明是呼叫出了問題還是本身被呼叫者系統出了問題。
		if(someResult == null) {
			// 代表呼叫失敗。
		} else if (!someResult.isExist) {
			// 呼叫雖然成功,但是所得到的a不存在。
		} else {
			// 呼叫成功,使用其值。
			a.value
		}

		// 我覺得更優美的寫法應該是
		try {
			if(!someResult.isExist){}
			else {}
		} catch(Exception e) {
			// 處理以NPE為代表的Exception.
		}

	}
}

複製程式碼
  1. 除了遵循相同的編碼規範和最佳實踐,我們還能夠如何保證程式碼上線更加安全。

人是會犯錯的。我認為光是強調規範並不能保證開發者一定100%遵守規範。同時,我也不認為這些規範就能覆蓋所有的開發質量問題。所以我認為還應該有更多的機制保證上線程式碼的質量。

TDD(Test-Driven Development)就是一個非常好的開發方法論。根據我自己對TDD的實踐,我覺得TDD的最大好處就是讓開發者知道自己正在做什麼。

拿上面的例子來說,如果應用了TDD,開發者在程式碼裡面所新增的任何一行程式碼都會得到Test-Case的檢驗。而且開發者會在編寫業務邏輯程式碼之前,認真思考我為什麼會加入這一行程式碼,這一行程式碼對我的邏輯有什麼作用。

這樣,就不至於會無腦地新增一些所謂的最佳實踐卻對業務邏輯沒有幫助的程式碼。

比如上面的例子,當我們的Legacy系統有類A與其子類B。當前執行良好,我們假定當前的程式碼都是經過Test過的。

那麼Legacy的當前程式碼snapshot如下:


@Setter
@Getter
class A {
  private boolean success;

  public void doSomeLogicAndSetSuccess() {
    // after doing some logic
    this.setSuccess(true);
  }
}

class B extends A {
}

class AUseCaseClass() {
	private B b = getBFromSomeService();

	public B doSomeLogicAndReturnB() {
		//after doing some logic 
		this.b.doSomeLogicAndSetSuccess();
		return this.b;
	}
}
//code 3
複製程式碼

這時有個開發因為有新的業務需求要修改code 3的程式碼,如果他是遵守TDD的話,他應該第一時間根據新的需求著手寫Test-Case。根據筆者自身的實踐經驗,如果對新的需求瞭解的越是不明白,越是寫不出Test-Case來,就更別說在原有程式碼上做改動了。就是這讓開發者思考業務需求的過程和強制先寫Test-Case的要求,讓其每一行寫入程式碼中的程式碼都不會成為廢程式碼和錯誤程式碼。

比如上面的事故,要是遵守TDD的話,這個開發者在新增如下程式碼之前,他至少會問自己,我為什麼新增success,難道A中沒有success嗎。同時,假如他沒有發現A中有success,他也會首先寫Test-Case,去驗證自己要新增的這一行是否會讓整個系統出現問題。

class B extends A {
  public Boolean success = null;
}
複製程式碼

結論

本文分析了筆者在工作中發現的事故,詳細描述了事故發生的起因,經過和結果;同時,根據事故的根本原因,從團隊未來預防同樣錯誤的角度和提升團隊整體程式碼質量的角度進行了深入的思考。筆者提出兩個反思結果:1. 根據需要,類屬性使用自定義資料型別(RPC方法返回值強制使用自定義資料型別)。2.使用TDD來保證程式碼質量(最好是做code-review的程式碼帶上其Test-Case,否則其實和裸奔沒有什麼區別)。

相關文章