코드 리뷰 - 완벽한 코드 없음 (1)
11993 단어 Code Review프로그래밍 습관
우리는 매일 코드를 써야 할 뿐만 아니라, 때때로 멈춰서 자신의 코드를 보아야 한다. 다음은 최근 프로젝트에서 코드 리뷰가 발견한 몇 가지 문제이다.좋은 코드 리뷰는 버그를 줄일 수 있을 뿐만 아니라 서로 배우는 과정이기도 하다.
1. 코드 배경: 데이터 유형의 전환에 부딪히는 경우가 많다. 특히string과number 간의 전환은 이럴 때 잘 처리하지 못하고 복잡한 업무 장면에 부딪히면 사용자의 행동이 통제할 수 없을 때 심지어 악의적인 방문을 받을 때 대량의 이상을 던지고 응용의 토출을 낮추기 쉽다.
if (NumberUtils.isNumber(product.getProductGroupId())) {
Integer productGroupId = Integer.valueOf(productSearch.getProductGroupId());
ProductGroupDTO productGroupDTO =productService.findProductGroupDTOByGroupId(productGroupId);
}이 코드에는 두 가지 문제가 있습니다.
1、org를 사용했습니다.apache.commons.lang.NumberUtils:
많은 시간 동안 우리가 클래스를 도입할 때 어떤 jar 패키지를 사용하는지 특별한 관심을 가지지 않는다.eclipse 등 도구에 대한 경고와 조언도 개의치 않는다.사실 우리는 일시적으로 오류가 없는 코드가 아니라 깨끗한 코드를 써야 한다.여기는 org를 써야 해요.apache.commons.lang.math.NumberUtils
2、Integer.value Of () 이 함수는 변환에 실패할 때 NumberFormatException에서 튀어나옵니다.이런 잘못된 데이터를 기록할 필요가 없는 장면은 이 문제를 피할 수 있다.
수정한 내용은 다음과 같습니다.
if (StringUtils.isNotBlank(product.getProductGroupId())) {
Integer authProductGroupId = NumberUtils.toInt(product.getproductGroupId(), -1);
ProductGroupDTO productGroupDTO =productService.findProductGroupDTOByGroupId(productGroupId);
}2. 코드 배경: 많은 경우에 우리는 매개 변수의 합법적인 판단을 해야 하지만 코드는 판단을 무시하기 쉬운 순서를 즐겁게 쓰는 것도 중요하다. 다음 코드를 보십시오.
if (companyId <= 0 || companyId == null) {
logger.error("companyId:" + (companyId == null ? "is null" : companyId.toString()) + ",please check company id.");
return true;
}foundbug를 사용하면 문제를 쉽게 발견할 수 있습니다:nullpointer early
수정 후:
if ( companyId == null || companyId <= 0 ) {
logger.error("companyId:" + (companyId == null ? "is null" : companyId.toString()) + ",please check company id.");
return true;
}3. 코드 배경: 같은 매개 변수의 합법성 판단으로 이번 오류는 더욱 은폐되었다.
if (map == null && map.size() == 0) {}수정 후:
if (map == null || map.size() == 0) {}4. 코드 배경: 많은 경우 한 방법에서 하나의 대상을 구축한다. 먼저 일정한 조건을 만족시키고 대상을 구축한다. 일부 속성을 설정하고 다른 부분의 조건을 만족시키며 다른 부분의 속성을 설정한다. 만약에 만족하지 않으면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()
}
}이 문제의 관건은 바로 전가인지 전인용인지 isHave ReportOne()==false를 가정하는 것이다.두 번째 함수에서 우리는 스스로 새로운 대상이 생겼다고 생각한다.그리고 다른 속성을 설정했다.하지만 리포트 뷰에서.setOther () 가 NullPointer Exception에서 벗어날 수 있도록
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 ;
}5.ibatis를 사용할 때 보통 xml,DAO,DO는 자동으로 생성된다. 이때 업데이트는 두 가지 방법이 있는데 하나는 모두 업데이트하는 것이고 다른 하나는null이 아닌 값만 업데이트하는 것이다.이 때 모든 업데이트를 사용할 때 사용자가 특정한 필드의 수정을 삭제하는 것을 허락하지 않는다면 두 번째 업데이트를 사용하는 것이 좋습니다.
6. 코드 배경:List
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 );
// ，
for (DataDto dataDto: reports) {
filterReport(hideSet, dataDto);
}
}
private void filterReport(Set<Integer> hideReports, DataDto report) {
if (report == null) {
return;
}
//
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);
}
}
}
//
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;
}
//
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 set
private void ListTOSet(Set<Integer> hideSet, List<DataItemPropertiesDO> list) {
for (DataItemPropertiesDO prop : list) {
hideSet.add(prop.getReportId());
}
}첫 번째 문제는 쉽게 발견할 수 있다. nullpointer는javaer에서 가장 흔하고 막을 수 없는 문제이다.많은 회사들이 어떤 방법도 null로 돌아가는 것을 허락하지 않기로 약속했다.문제는 바로
Set<Integer> hideSet = new HashSet<Integer>((hideReportsOne.size() + hideReportsTwo.size()) * 4 / 3);여기의hideReportsOne,hideReportsTwo는null일 수 있습니다.서비스를 제공하는 측이 가능한 한 null로 돌아오지 않으려면 Collections를 고려할 수 있습니다.null 대신 emptyList();다른 한편으로는 영원히 다른 사람의 정확성에 의존해서는 안 된다.
두 번째 문제도 어렵지 않다. list All Hide Reports One과list All Hide Reports Two는 매우 유사하지만 조회하는 키가 같지 않다. 만약에 나중에 필터가 필요한 데이터를 추가하면 현재의 방식은 반드시 비슷한 방법을 추가해야 한다. 이를 통해 확장성이 매우 좋지 않다는 것을 알 수 있다.
하나의 시나리오:
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);
}
}
}이 내용에 흥미가 있습니까?
현재 기사가 여러분의 문제를 해결하지 못하는 경우 AI 엔진은 머신러닝 분석(스마트 모델이 방금 만들어져 부정확한 경우가 있을 수 있음)을 통해 가장 유사한 기사를 추천합니다:
code review 작업5가지 이점: 1. 코드의 스타일이 우수하고 주석이 많지 않지만 필요한 주석이 있다. 2. 코드의 건장성이 양호하여 여러 가지 특수한 상황에 대해 오류 처리를 하고 오류 정보를 출력할 수 있다. 3. Main 함수는...
텍스트를 자유롭게 공유하거나 복사할 수 있습니다.하지만 이 문서의 URL은 참조 URL로 남겨 두십시오.
CC BY-SA 2.5, CC BY-SA 3.0 및 CC BY-SA 4.0에 따라 라이센스가 부여됩니다.
좋은 웹페이지 즐겨찾기
