這是一個老題目囉,會再次寫是因為現在又多了一些經驗,有了新的想法。getter setter 這類的 property accessor 是相當常見的程式手段, 很多語言甚至內建。開發期間,我大部份的時間都在避免使用 getter setter,盡可能採用別的寫法,也希望別的開發人員能 follow。不過,有人反駁說,getter/setter 在語言或IDE都內建了,即然創作者設計出這個功能來,不就是要鼓勵開發者運用這個功能嗎?為何有方便的工具/寫法卻不去用咧!?針對這種問題,請各位看一下下面的例子:
public void a9981ddkkkadfdee(String ddfjkq) {...} public void saveFile(String filename) {...}
So, 哪一個 method 比較容易維護、好用?這還用問嗎?這個道理很淺顯的 -- 語言提供給我們語法、給我們自由發揮,但不代表可以愛怎麼用就怎麼用。就像菜刀可以切一盤好菜、也可殺人一樣,工具用錯了就是個災難。Getter and setter 是一項工具,可惜太容易被誤用,反而變成 bug 的根源。
實例
用實例解說最有說服力,讓我們開始吧:今天客戶有一個需求,希望統計用戶到網站進行購物的行為 -- 用戶如果完成了購物的程序,就給予正面的評價;如果用戶買到一半,購物車放了一堆東西,不刷卡半途就跑掉了,這時就扣用戶的評價。工程師聽到需求之後,發現執行上有困難,因為用戶直接關閉網頁時,server 並不會知道,所以沒辦法馬上做負評的動作。與客戶討論後,雙方都同意採取變通的辦法:當用戶開始購物後,先預扣他的評價,直到他完成購物後才補回。如果他沒完成購物,則預扣的部份可以在他下次進來購物時再扣回,或是用 batch 在後端每天跑一次。
工程師很快就設計出 table:
UserRating { id int, user_id int, last_modified_date date, rating int, -- 目前評價 withhold int -- 預扣值 }
而他開始在他的 shopService 這個 facade 加上這個新功能:
@Transactional public class ShopServiceImpl implements ShopService { public ShoppingCart startShopping(User user) { UserRating userRating = createOrLoadRating(user); //如果之前有預扣沒清除,就先扣掉 if(userRating.getWithhold() > 0) { userRating.setRating( userRating.getRating()-userRating.getWithhold()); } //然後再針對這次的購買預扣 2 點 userRating.setWithhold(2); userRatingDao.save(userRating); //.... create shoppingCart return shoppingCart ; } public void endShopping(ShoppingCart shoppingCart) { UserRating userRating = createOrLoadRating(shoppingCart.getBuyer()); //完成購買程序,清除預扣 userRating.setWithhold(0); //替買家加一點評價吧: userRating.setRating(userRating.getRating() + 1); userRatingDao.save(userRating); //.... complete shoppingCart saveOrder(shoppingCart); } }
按需求,我們在開始購買 startShopping(), 和結清 endShopping() 時,安插了對評價的加減。開始購買時會先預扣個評價, 購買完成後則會把預扣值清掉。如果開始購買前,發現用戶還保有預扣值,表示他之前沒有完成整個購買程序,這時我們要先扣回來。
ok, 到目前為止沒什麼問題。這種寫程式的方法是 Transaction Script,把要做的事通通堆在 facade 的 method 裡直接一口氣做完就是了, 這是最多人寫程式的方式,但其實處處是破洞。第一個問題就是 facade 裡的 method 越來越長了,跟真正購買無關的事多佔據了好幾行。這裡已經把 database 的操作搬到了 dao 去了,不然更是累贅。第二個是 rating / withhold 的值沒有做任何保護,任何拿到 userRating 物件的人都可以恣意修改。而且 withhold 和 rating 間值的變化必須按照一定的程序來做的,這個也沒有保護到。
有經驗的 Java developer,遇到這些問題,馬上善用 IDE 強大的 extract method refactoring,整理了一下程式碼:
@Transactional public class ShopServiceImpl implements ShopService { public ShoppingCart startShopping(User user) { withholdUserRating(user); //.... create shoppingCart return shoppingCart ; } public void endShopping(ShoppingCart shoppingCart) { giveUserRating(shoppingCart.getBuyer()); //.... complete shoppingCart saveOrder(shoppingCart); } private void withholdUserRating(User user) { UserRating userRating = createOrLoadRating(user); if( hasPreviousWithhold(userRating) ) { updateRating(userRating, -userRating.getWithhold()); } //然後再針對這次的購買預扣 2 點 updateWithhold(userRating, 2); userRatingDao.save(userRating); } private boolean hasPreviousWithhold(UserRating userRating) { return userRating.getWithhold() > 0; } private void giveUserRating(User user) { UserRating userRating = createOrLoadRating(shoppingCart.getBuyer()); //完成購買程序,清除預扣 updateWithhold(userRating, 0); //替買家加一點評價吧: updateRating(userRating, 1); userRatingDao.save(userRating); } private void updateRating(UserRating userRating, int differece) { userRating.setRating( userRating.getRating() + differece ); //最低為 0 if(userRating.getRating() < 0) { userRating.setRating(0); } } private void updateWithhold(UserRating userRating, int amount) { assert amount >= 0 : "預扣值不可為負" ; userRating.setWithhold(amount); } }
整理過後,startShopping, endShopping 變得比較清爽了,然後 rating 有了最小值的保護 (最小是0), withhold 的值也加了 contract 的保護 -- 不可為負值,這樣好像還不賴,工程師 refactoring 完之後,收工換做下一個 task。然而.... 挑戰是從新的需求來才開始的....
一個月過後...
新功能上線一個月之後,客戶說在下次購買前才將預扣值扣回太慢了,希望可以在用戶一登入,馬上就將預扣扣回,這樣用戶一進來就可以依照最新的評價值,推薦不同的產品。工程師 B 是個 database guy,在收到這個 task 之後,他先去看了 table。發現有 table user_rating 和 withhold 這個欄位,也很快的找到對應的 UserRating class,而且還有 Dao 可用耶,興沖沖的在 UserService login 的地方就加了這幾行:
@Transactional public class UserServiceImpl implements UserService { public void login(User user) { UserRating userRating = createOrLoadRating(user); if( userRating.getWithhold() > 0 ) { userRating.setRating( userRating.getRating() - userRating.getWithhold()); userRating.setWithhold(0); userRatingDao.save(userRating); } //do login.... } }
well, 這回工程師 B 犯了程式碼重覆的毛病,不過他可不知道程式有重覆到,他自己測試時也都正常 (他反覆測了二、三次)。但是上線後的隔天.... 客戶發現有用戶的評價開始變成負值了,網頁更出現奇怪的事,像是推薦到不該推薦的東西。而另一頭用評價來算 VIP 客戶折扣的地方,負評的人通通誤判成 VIP,一堆人莫名奇妙打六折,於是另一場 鄉民大戰 DELL 的戲碼又再度上演....
Getter and setter are evil
第一位工程師當初在設計時,並沒有去思考物件封裝的問題,他習慣性的就寫了 getter setter,將 UserRating 內部所有的欄位全部暴露出來,而他所做的有關預扣加評價的邏輯,以及評價最低值的保護,則放在完全不相干的 ShopServiceImpl 裡。你說他做錯了什麼嗎?看看上面的程式碼,其實也十幾行而已。十幾行的程式他一下就寫完了,又加了中文註解。而且他也遵循了 "用最簡單的方法完成任務" 的精神,不多做無謂的設計。他會想這樣後續維護不會很難吧?但是以結果論,的的確確是造成了災難...
怎麼辦?
答案在被大家遺忘在學校中的 OOP 裡 -- 將資料封裝並且定義物件的行為。
也在 FP (functional programming) 裡 -- 不會改變的資料最安全 (prefer Immutability)
接下來將移除萬惡 getter setter,按照上述的精神,示範一種較為強韌的設計,這並不是唯一解,也不是最好解,只是我個人開始用這樣的 style 寫程式,分享給大家參考:
@Transactional public class ShopServiceImpl implements ShopService { public ShoppingCart startShopping(User user) { UserRating userRating = createOrLoadRating(user); userRating.prepareWithhold(2); userRatingDao.save(userRating); //.... create shoppingCart return shoppingCart ; } public void endShopping(ShoppingCart shoppingCart) { UserRating userRating = createOrLoadRating(shoppingCart.getBuyer()); userRating.giveRating(1); userRatingDao.save(userRating); //.... complete shoppingCart saveOrder(shoppingCart); } }
我們給了 UserRating 兩個新的行為,一是 prepareWithhold(int amount) 二是 giveRating(int amount),專門打造給 ShopService 呼叫。這樣 ShopService 一樣很乾淨,而且也清楚在幹嘛 (不用寫什麼註解了),接下來看一下大改造之後的 UserRating:
@Entity public class UserRating { @Id private Long id; @ManyToOne(optional = false) private User user; @Basic(optional = false) private Date lastModifiedDate; @Basic(optional = false) @Embedded private WithholdableRating withholdableRating; public UserRating(User user) { this.user = user; this.withholdableRating = WithholdableRating.INITIAL; this.lastModifiedDate = new Date(); } public void prepareWithhold(final int amount) { withholdableRating = withholdableRating .deductPreviousWithhold() .withhold(amount); lastModifiedDate = new Date(); } public void giveRating(final int amount) { withholdableRating = withholdableRating .returnPreviousWithhold() .giveRating(amount); lastModifiedDate = new Date(); } public User getUser() { return user; } public int getRating() { return withholdableRating.getRating(); } }
@Embeddable public class WithholdableRating implements Serializable { @Column(name="rating", nullable=false) private int rating; @Column(name="withhold", nullable=false) private int withhold; static final WithholdableRating INITIAL = new WithholdableRating(100, 0) ; WithholdableRating() { //for JPA } private WithholdableRating(final int rating, final int withhold) { assert withhold >= 0 : "withhold must be >=0" this.rating = Math.max(0, rating)); this.withhold = withhold; } //扣掉先前預扣 public WithholdableRating deductPreviousWithhold() { return new WithholdableRating(rating - withhold, 0); } public WithholdableRating giveRating(final int amount) { assert amount >= 0 : "giveRating must be >= 0" ; return new WithholdableRating(rating + amount, withhold); } //清除舊的預扣 public WithholdableRating returnPreviousWithhold() { return new WithholdableRating(rating, 0); } public WithholdableRating withhold(final int amount) { assert withhold == 0 : "should not withhold again"; return new WithholdableRating(rating, amount); } int getRating() { return rating; } //omit equals(), hashCode(), and toString() }
UserRating 是 Entity,有 id,這個跟原來一樣。不過原來的版本他是有五個欄位的,但這裡將預扣值 (withhold) 和評價值 (rating) 這兩個欄位包在一塊,變成一個獨立的 value object 'WithholdableRating' 了。為何要特意包在一起?原因是我觀察到 withhold 和 rating 這兩個值,常常一起改變,它們必須按照某種規則操作,而且其值域也有所限制。我可以在 UserRating 裡寫一堆 if 來管理這兩個值的關係。 但是東一個 if 西一個 if 還是很亂,沒有辦法讓程式 "speak itself"。這裡特意將 WithholdableRating 設計成 Immutable object ,每一次改變值就重新產生新物件,而且它的 method 都有對應到需求 -- '預扣'、'扣回舊的預扣'、'清除舊的預扣'、'增加評價'。 如果讀者注意的話,這四個字眼在我們上面的討論中已出現過幾次。
上面蓼蓼數行的 WithholdableRating 可不是隨便寫寫的,這個物件是不會 '崩壞' 的。 首先,它無法被 construct。我們只能拿到 INITIAL 這個 constant 開始操作,因此在程式中無法建立任何不按規矩的 WithholdableRating。 其次 withhold(amount) 這個 method 不能連續呼叫兩次,這是另一層的防衛。另外也限制 withhold 和 rating 這兩個值不可為負值。 而 rating 的最低值則保護在 private constructor 裡。WithholdableRating非常的小且單純,可以想像它的 unit test 有多好寫了吧。最後一點,眼尖的讀者應該已經看出其實還是有幾個漏洞,第一個是 default constructor,另一個是 rating 和 withold 這兩個欄位不是 final。所以嚴格來說它並不是真正的 Immutable Object --- 這一切都是向 JPA/Hibernate 妥協....
Anyway, WithholdableRating 實際用起來怎麼樣呢?看看 UserRating.prepareWithhold() 吧,裡面寫了一段 '話' :
withholdableRating = withholdableRating.deductPreviousWithhold().withhold(amount)
'先扣回舊的預扣、再做預扣'
另一個 method userRating.giveRating() 則說了 -- '清除舊的預扣、再給予評價':
withholdableRating = withholdableRating.returnPreviousWithhold().giveRating(amount);
像白話文一般的程式,簡單到了一個極致。
好了,總結一下,新版的程式完成幾個目標 -- 程式碼自己說了自己做了什麼;不用註解;沒有散在各地的 if;WithholdableRating 牢不可破;Easier Unit Testing。
那麼,讓我們時光倒流。當工程師 B 遇到這樣的程式,他這一次會怎麼處理呢?首先他也是找到了資料庫 rating 和 withhold 的欄位,不過他找到 UserRating 時,大概會愣個一下吧?居然只有兩個 method 可用,而且看名字也不是他想要的,他想想只好自己動手加吧。往 UserRating 裡面一挖,又看到 WithholdableRating,這回他愣比較久了。如果他之前沒碰過 Immutable 的概念,那他不是馬上找救兵,就是會破口大罵吧?不論如何,在 withholdableRating 上他可以看到非常白話的 method: deductPreviousWithhold()。這是他想要的,所以他依樣寫了幾行:
@Transactional public class UserServiceImpl implements UserService { public void login(User user) { //程式碼重用,而且不會 東 if 西 if UserRating userRating = createOrLoadRating(user); userRating.deductPreviousWithhold(); userRatingDao.save(userRating); //do login.... } } @Entity public class UserRating { //... omit //在 UserRating 上加上第三個行為 //工程師 B 不小心會錯意或搞錯也無所謂, //基本上 withholdableRating 牢不可破,不會留下任何錯誤的狀態。 public void deductPreviousWithhold() { withholdableRating = withholdableRating.deductPreviousWithhold(); lastModifiedDate = new Date(); } //... omit }
這一次,大家晚上都睡了個好覺....