?
我們不但要每天寫(xiě)代碼，更應(yīng)該時(shí)時(shí)停下來(lái)去看看自己的代碼：下面是最近項(xiàng)目中code review 發(fā)現(xiàn)的一些問(wèn)題；好的code review 不僅可以減少bug，更加是一個(gè)互相學(xué)習(xí)的過(guò)程：
?
?
一、代碼背景：很多時(shí)候都會(huì)碰到數(shù)據(jù)類(lèi)型的轉(zhuǎn)換，特別是string 和number之間的轉(zhuǎn)換，這個(gè)時(shí)候處理的不好，碰上復(fù)雜的業(yè)務(wù)場(chǎng)景，在用戶(hù)行為不可控的時(shí)候，甚至遭到惡意訪(fǎng)問(wèn)的時(shí)候，很容易拋出大量異常，降低應(yīng)用的吐出；
?
?
if (NumberUtils.isNumber(product.getProductGroupId())) {
Integer productGroupId = Integer.valueOf(productSearch.getProductGroupId());
ProductGroupDTO productGroupDTO =productService.findProductGroupDTOByGroupId(productGroupId);
}
?
這段代碼有兩個(gè)問(wèn)題：
?
? 1、使用了?org.apache.commons.lang.NumberUtils：
很多時(shí)間我們?cè)谝腩?lèi)的時(shí)候，不會(huì)特別關(guān)心到底是用的哪一個(gè)jar包里面的；對(duì)于eclipse等工具的警告和建議也不會(huì)去在意；事實(shí)上我們應(yīng)該寫(xiě)一段干凈的代碼，而不是暫時(shí)沒(méi)有錯(cuò)誤的代碼；這里應(yīng)該用?org.apache.commons.lang.math.NumberUtils
?
? 2、Integer.valueOf()這個(gè)函數(shù)當(dāng)轉(zhuǎn)換失敗的時(shí)候會(huì)跑出NumberFormatException;對(duì)于這里并不需要取記錄這種錯(cuò)誤數(shù)據(jù)的場(chǎng)景，可以很好的避免這個(gè)問(wèn)題：
修改后如下：
?
?if (StringUtils.isNotBlank(product.getProductGroupId())) {
Integer authProductGroupId = NumberUtils.toInt(product.getproductGroupId(), -1);
ProductGroupDTO productGroupDTO =productService.findProductGroupDTOByGroupId(productGroupId);
}
??
二、代碼背景：很多時(shí)候我們都需要做一些參數(shù)合法性的判斷，但是代碼寫(xiě)的歡了容易忽略判斷的順序有時(shí)候也是很重要的，見(jiàn)下面代碼：
?
?
?
if (companyId <= 0 || companyId == null) {
logger.error("companyId:" + (companyId == null ? "is null" : companyId.toString()) + ",please check company id.");
return true;
}
?
?
?使用foundbug 很容易發(fā)現(xiàn)問(wèn)題：nullpointer early
?
?修改后：
?
?
if ( companyId == null || companyId <= 0 ) {
logger.error("companyId:" + (companyId == null ? "is null" : companyId.toString()) + ",please check company id.");
return true;
}
?
?
三、代碼背景：同樣是參數(shù)合法性的判斷，這次的錯(cuò)誤就更加隱蔽了：
?
?
if (map == null && map.size() == 0) {}
?
?
修改后的：
?
?
if (map == null || map.size() == 0) {}
?
?
?四、 代碼背景：很多時(shí)候會(huì)在一個(gè)方法里面去構(gòu)建一個(gè)對(duì)象，首先是滿(mǎn)足一定的條件，構(gòu)建對(duì)象，設(shè)置部分屬性，滿(mǎn)足另外一個(gè)部分條件，在設(shè)置另外一部分屬性，如果都不滿(mǎn)足，返回null；看下面代碼：
?
?
public ReportView buildReportView (){
ReportView reportView = null;
if (isHaveReportOne()) {
reportView = buildReportOneView(ReportDTOOne);
}
if ((isHaveReportTwo()) {
setReportTwoDTO(reportView , ReportDTOTwo);
reportView.setOther();
}
?
private void setReportTwoDTO(ReportView reportView , ReportTwoDTO reportTwoDTO){
if(reportView = null)
{
reportView = new reportView()
}
}
?
? 這個(gè)問(wèn)題的關(guān)鍵其實(shí)就是：傳值還是傳引用，假設(shè) isHaveReportOne() ==false;在第二個(gè)函數(shù)中我們自以為new 了新的對(duì)象；并且設(shè)置了其他的屬性；但是在reportView.setOther()確任然跑出NullPointerException,
?
?
public ReportView buildReportView (){
ReportView reportView = null;
if (isHaveReportOne()) {
reportView = buildReportOneView(ReportDTOOne);
}
if ((isHaveReportTwo()) {
reportView = setReportTwoDTO(reportView , ReportDTOTwo);
reportView.setOther();
}
?
?
?
private ReportView setReportTwoDTO(ReportView reportView , ReportTwoDTO reportTwoDTO){
if(reportView = null)
{
reportView = new reportView()
}
return reportView ;
}
?
?
五、在使用ibatis的時(shí)候，通常xml，DAO，DO都是自動(dòng)生成的，這個(gè)時(shí)候update會(huì)有兩個(gè)方法，一種是全部更新，另外一個(gè)只是更新不為null的值。這個(gè)時(shí)候使用全部更新要特別注意，如果是不允許用戶(hù)刪除某個(gè)字段的修改，最好使用第二種更新。
?
?
六、代碼背景：List<DataDto> reports 保存了從DB取出的數(shù)據(jù)，但是由于涉及到機(jī)密，需要將一部分值替換為一個(gè)常數(shù)；
?
?
public void filterHideItem(List<DataDto> reports) {
if (reports == null || reports.isEmpty()) {
return;
}
List<DataItemPropertiesDO> hideReportsOne = listAllHideReportsOne();
List<DataItemPropertiesDO> hideReportsTwo = listAllHideReportsTwo();
Set<Integer> hideSet = new HashSet<Integer>(( hideReportsOne .size() + hideReportsTwo.size()) * 4 / 3);
ListTOSet(hideSet, hideReportsOne );
ListTOSet(hideSet, hideReportsTwo );
// 循環(huán)比較如果是在需要被屏蔽的集合里面，則替換為一個(gè)常數(shù)
for (DataDto dataDto: reports) {
filterReport(hideSet, dataDto);
}
}
private void filterReport(Set<Integer> hideReports, DataDto report) {
if (report == null) {
return;
}
//需要過(guò)濾的報(bào)告數(shù)據(jù)
List<NormalDataDto> list = report.getNormalReports();
for (NormalDataDto normalDto : list) {
if (hideReports.contains(normalDto.getId())) {
normalDto.getSummaryKeyValueMap().put(Constants.KEY_ONE,
Constants.VALUE);
normalDto.getSummaryKeyValueMap().put(Constants.KEY_TWO,
Constants.VALUE);
}
}
}
// 獲取 第一種類(lèi)型需要被屏蔽的數(shù)據(jù)
private List<DataItemPropertiesDO> listAllHideReportsOne() {
DataItemPropertiesDO param = new DataItemPropertiesDO();
param.setKey(Constants.KEY_ONE);
param.setName(Constants.ATTRIBUTE_HIDE);
param.setValue(Constants.ATTRIBUTE_HIDE_TRUE_VALUE);
List<DataItemPropertiesDO> list = service.listItemPropertiesByParam(param);
if (list == null || list.isEmpty()) {
return null;
}
return list;
}
//獲取 另外一種需要被屏蔽的數(shù)據(jù)
private List<DataItemPropertiesDO> listAllHideReportsTwo() {
DataItemPropertiesDO param = new DataItemPropertiesDO();
param.setKey(Constants.KEY_ONE);
param.setName(Constants.ATTRIBUTE_HIDE);
param.setValue(Constants.ATTRIBUTE_HIDE_TRUE_VALUE);
List<DataItemPropertiesDO> list = service.listItemPropertiesByParam(param);
if (list == null || list.isEmpty()) {
return null;
}
return list;
}
// 將List 轉(zhuǎn)換為 set
private void ListTOSet(Set<Integer> hideSet, List<DataItemPropertiesDO> list) {
for (DataItemPropertiesDO prop : list) {
hideSet.add(prop.getReportId());
}
}
?
?
?
第一個(gè)問(wèn)題很容易發(fā)現(xiàn)：nullpointer 是javaer最常見(jiàn)也最防不勝防的問(wèn)題；很多公司更是作為一個(gè)約定不允許任何方法返回null；問(wèn)題就出現(xiàn)在
?
?
Set<Integer> hideSet = new HashSet<Integer>((hideReportsOne.size() + hideReportsTwo.size()) * 4 / 3);
?
?
?? 這里的
hideReportsOne?，hideReportsTwo?可能為null；?一方面提供服務(wù)的一方 要盡可能的不返回 null 可以考慮 ?Collections.emptyList() 代替 null；另一方面 ?永遠(yuǎn)不能依賴(lài)別人的正確性，
?
?第二個(gè)問(wèn)題也不難，? 可以發(fā)現(xiàn) listAllHideReportsOne 和 listAllHideReportsTwo 是非常類(lèi)似，只是查詢(xún)的key不相同,如果將來(lái)在新增一個(gè)需要過(guò)濾的數(shù)據(jù)，現(xiàn)在的方式必然要再新增一個(gè)類(lèi)似的方法，可見(jiàn)可擴(kuò)展性非常不好：
?
一種方案：
?
?
private List<String> hideItemKeys;
public ItemHideFilterProcessor() {
hideItemKeys = new ArrayList<String>(2);
hideItemKeys.add(Constants.KEY_ONE);
hideItemKeys.add(Constants.KEY_TWO);
}
public void filterHideItem(List<DataDto> reports) {
if (reports == null || reports.isEmpty()) {
return;
}
List<ItemHideReport> itemHideReportList = buildItemHideReports();
if (itemHideReportList.isEmpty()) {
return;
}
for (DataDto dataDto : reports) {
filterReport(itemHideReportList, dataDto );
}
}
private void filterReport(List<ItemHideReport> hideReportSetList, DataDto report) {
if (report == null) {
return;
}
List<NormalDataDto> list = report.getNormalReports();
for (NormalDataDto normalDto : list) {
for (ItemHideReport hideReport : hideReportSetList) {
if (hideReport.isHide(normalDto.getId())) {
normalDto.getSummaryKeyValueMap().put(hideReport.getItemKey(),
CertifiedReportConstants.CONFIDENTIAL);
}
}
}
}
private List<ItemHideReport> buildItemHideReports() {
List<ItemHideReport> list = new ArrayList<ItemHideReport>(hideItemKeys.size());
for (String hideKey : hideItemKeys) {
Set<Integer> reportSet = listAllItemHideReportsByKey(hideKey);
if (reportSet != null) {
list.add(new ItemHideReport(hideKey, reportSet));
}
}
return list;
}
private Set<Integer> listAllItemHideReportsByKey(String key) {
DataItemPropertiesDO param = new DataItemPropertiesDO();
param.setKey(key);
param.setName(Constants.ATTRIBUTE_HIDE);
param.setValue(Constants.ATTRIBUTE_HIDE_TRUE_VALUE);
List<DataItemPropertiesDO> list = itemService.listItemPropertiesByParam(param);
if (list == null || list.isEmpty()) {
return null;
}
Set<Integer> set = new HashSet<Integer>(list.size() * 4 / 3);
for (DataItemPropertiesDO prop : list) {
set.add(prop.getReportId());
}
return set;
}
class ItemHideReport {
String itemKey;
Set<Integer> hideReports;
public ItemHideReport(String itemKey, Set<Integer> hideReports) {
this.itemKey = itemKey;
this.hideReports = hideReports;
}
public String getItemKey() {
return itemKey;
}
public boolean isHide(Integer reportId) {
return hideReports.contains(reportId);
}
}
}