今天Code review會議同事W對下面的程式碼提出疑問。
public void doSomeService(DemoReq req, ... ) {
DemoContainer container = new DemoContainer();
container.setDemoReq(req);
// 下面是覺得有問題的程式碼
DemoSetting setting = otherService.createDemoSetting(container);
container.setDemoSetting(setting);
...
}
同事W的疑問是為什麼otherService.createDemoSetting()建立DemoSetting物件時傳入DemoContainer物件，然後又把建立好的DemoSetting物件塞回DemoContainer裡，這段程式碼令人困惑。
從以上現有的程式碼可以推測DemoContainer的結構如下。
DemoContainer
public class DemoContainer {
private DemoReq demoReq;
private DemoSetting demoSetting;
...
}
也就是DemoContainer中有成員變數DemoSetting欄位，初始為null。
從程式碼來看otherService.createDemoSetting()產生DemoSetting的來源為DemoContainer，所以同事W困惑的地方在於DemoContainer自己產生的東西（也就是DemoSetting）又被放回DemoContainer本身，既然如此container.setDemoSetting(setting)這段程式碼何不包在createDemoSetting()就好呢？W認為現在這樣的寫法會令人感到疑惑。
同事解釋如果包進createDemoSetting()裡，就不會知道DemoSetting已經在createDemoSetting()裡偷偷被塞進了DemoContainer，擔心side effect的問題。
那如果修改一下命名改成otherService.setDemoSetting(container)應該就能解決了。從命名setDemoSetting()搭配傳入的物件來看，即可推測此方法會建立DemoSetting物件並放入傳入的物件中。
public void doSomeService(DemoReq req, ... ) {
DemoContainer container = new DemoContainer();
container.setDemoReq(req);
otherService.setDemoSetting(container);
...
}
或是建立另外一個工廠類DemoContainerFactory.create(DemoReq req)來產生含有DemoSetting的DemoContainer物件。
public void doSomeService(DemoReq req, ... ) {
DemoContainer container = DemoContainerFactory.create(req);
...
}
參考：
沒有留言:
張貼留言