08 November 2009

這是一個老題目囉,會再次寫是因為現在又多了一些經驗,有了新的想法。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
}

這一次,大家晚上都睡了個好覺....


回響

可以用 Tag <I>、<B>,程式碼請用 <PRE>