今天同事對我在建構式中轉寫邏輯的寫法提出疑問如下。
下面的Employee是我寫得一個用來映射EMPLOYEE資料表的Entity類別。
@Entity
public class Employee {
@Id
private Long id;
@Column
private String name; // 姓名
@Column
private String encryptIdentityNumber; // 加密的身分證號
@Column
private Integer age; // 年紀
public Employee(String name, String identityNumber) {
this.name = name;
this.encryptIdentityNumber = EncryptUtil.encrypt(identityNumber); // <-- 問題點
}
// getters and setters...
}
同事認為Entity類或DTO類的建構式中不應該做邏輯處理，而我卻在理面將身分證號進行了加密EncryptUtil.encrypt(identityNumber)，這段加密身分證號的邏輯應該擺在外面才對。
同事認為應該改成這樣。
@Service
public class EmployeeManagementService {
@Autowired
private EmployeeDao employeeDao;
public void add(EmployeeDto dto) {
Employee entity = new Employee();
entity.setEncryptIdentityNumber(
EncryptUtil.encrypt(dto.getIdentityNumber())); // 加密的邏輯應該放在外面
entity.setAge(dto.getAge());
employeeDao.add(entity);
}
}
在建構式中處理業務邏輯可能造成以下問題，
第一個問題是建構式難以重用。這個建構式只有在輸入參數為未加密的身分證號時才用得到，如果已經有了加密的身分證號，這個建構式就無法用了，簡單說就是建構式的重用性很低。
第二個問題是無法從外部看出這個建構式幹了什麼，因為建構式並無法像方法那樣可以任意命名，而這個建構式卻在理面把身分證號給加密了，如果沒進來看還真不知道。
第三個問題是單元測試變得麻煩，例如撰寫EmployeeManagementService.add()的Unit Test程式時無法直接對加密工具EncryptUtilstub，還得另外對Employee做Mock。
第四個問題是如果EncryptUtil.encrypt(String str)因算法改變需要多個參數EncryptUtil.encrypt(String str, String salt)，那這個建構式也得被修改，違反開放封閉原則 Open-Closed Principle (OCP)。
第五個問題是違反類別的單一職責原則Single Responsibility Principle (SRP)，如果Employee的角色是做為映射資料表的類別，那它應該就做這件事就好，不應該參雜有的沒的其他邏輯。
而我原本的想法是既然身分證號都要經過加密後存入，那勢必在建立Employee時一定要做這件事，也就是加密的身分證號與Employee是緊密相關的，因此我才會把加密身分證的邏輯寫在建構式中，但卻沒有思考到以上造成的缺點。
我原本也是一直認為Model或POJO這種容器類只應該存在沒有任何邏輯的getter，setter與constructor，但這半年來又反思業務邏輯有時內聚在Model裡似乎更好維護，及才能方便利用各種設計模式。隔天和同事討論後才恍然大悟應該透過另一個叫Model類別來達成，而不是直接用Entity類。
非常感謝同事的提醒，讓我有機會考這個問題，明天要去跟他磕頭致謝。
參考：
沒有留言:
張貼留言