code review --没有完美的代码(一)
?
我们不但要每天写代码,更应该时时停下来去看看自己的代码:下面是最近项目中code review 发现的一些问题;好的code review 不仅可以减少bug,更加是一个互相学习的过程:
?
?
一、代码背景:很多时候都会碰到数据类型的转换,特别是string 和number之间的转换,这个时候处理的不好,碰上复杂的业务场景,在用户行为不可控的时候,甚至遭到恶意访问的时候,很容易抛出大量异常,降低应用的吐出;
?
?
??
?使用foundbug 很容易发现问题:nullpointer early
?
?修改后:
?
?
??
三、代码背景:同样是参数合法性的判断,这次的错误就更加隐蔽了:
?
?
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); } }}