2014년 10월 22일 수요일

Code Review 확인사항


  • 변하는 값이 static이면 thread safe한지 고민해야 한다.
  • public / private 혼동, 추상 Excpetion 안된다.
  • 코드가 중복되면 메소드로 따로 빼고 그것 마자 중복이면 추상 클래스로 빼야 한다.
  • 상황이 중복되고 여러가지가 있을 경우 배열형태로 하고 iteration 한다.(Unit Test)
  • 유틸성 클래스는 static으로 구현(속성이 필요없고, 객체 생성이 없다)
  • 임시코드일 경우 주석 처리 후 "//TODO" 추가한다.
  • setter 관련 메소드는 true, false를 반환하지 않는 void 형태여야 하고 false가 아닌 exception을 던져야 한다.
  • 코드를 읽고 내려갈 수 있도록 메소드, 변수 이름이 명확해야 한다.
  • 개발된 코드가 외부에서 호출될 때 사용성이 좋은지 확인해야 한다.
  • 로깅을 위한 코드라면 if(logger.isDebugEnabled()){.....} 를 사용하자
  • 로그에서 출력할 변수명은 진짜 변수명을 쓰자
  • 구현되지 않은 메소드를 만들 땐 "throw new RuntimeException("not implement")"
  • class에 속성이 있으면 init 확인하자
  • 파라메타가 많은 경우 메소드 호출 전에 미리 선언하자 - 가독성
  • Unit Test에 속성은 웬만하면 두지 말자(의존성 제거)
  • 연관된 메소드는 동일한 위치에 두자. - 가독성
  • 클래스 별로 해야 할 일을 구분하는게 중요하다
  • 메소드의 이름은 동사로 시작하고 클래스의 이름은 명사로 시작하자
  • //TODO는 보이는 즉시 작업하고 커밋하자 -> 10개 이상 넘어가면 멈추고 TODO를 털어내는 작업(별도의 Task)을 해야 한다.
  • pre condition 먼저 체크하자. if 구문이 3개 이상이면 리펙토링 하자
  • 메소드는 입력과 출력의 관계가 명확해야 한다.(불필요한 속성은 제거하자)
  • ().().().() 코드의 경우 에러 발생 시 어디에서 에러가 발생하는지 확인하기가 힘들다. 하나씩 체크하자
  • TestCase는 로그를 남기지 않는다. System.out.println을 사용하자
  • array[0][1]을 테스트할 경우 그게 어떤 값인지 눈에 들어오지 않는다. 명확하게 변수로 처리하여 대입하자
  • logger의 레벨은 System 기준으로 한다. - exception 처리가 제대로 되었다면 그건 error 레벨이 아니다. info 정도...
  • read only 기능인데 특정 메소드를 사용하면서 데이터가 가공된다면(write) 체크하려는 데이터는 clone을 사용하자.
  • 사용하지 않는 주석은 삭제하자
  • bean 클래스에서 매개변수를 validation 체크 하려면 모든 bean 클래서에서 하던지 안하던지 일관성 있게 하자. -> validate는 보통 로직이므로 bean에서는 하지 않는다.
  • 구조체의 toString 비교는 지양하자.
  • 본 코드에서 생성되는 데이터와 테스트 코드에서 사용된 데이터를 같이 비교하지 말자. 데이터 자체가 다르다.
  • 클래스 내부에서 매개변수 처리하여 넘겨준 데이터들은 굳이 validate 하지 않는다. 단, 외부에서 넘어오는 매개변수는 validate 하자.
  • 버그 발생 시 testcase로 만들어서 관리하자. -> 해당 버그는 같은 증상으로 재현되지 않는다.
  • 메소드 호출 여부를 확인하는 테스트 코드(불린 변수)가 본 코드에 삽입될 때에는 변수이름(xxxOnlyForTest)과 접근제한자(private)를 두어 테스트케이스에서만 사용하자.


댓글 없음:

댓글 쓰기