포스트

2021-02-26-TIL

체스 미션 4 코드 리뷰(by Tree)에서 나온 의견을 리뷰어별로 정리하고, 리뷰에 나온 주제 몇 가지를 찾아봤다.


코드 리뷰: 미션 4

[Tree]

  • Board
    • 메서드 이름으로 충분히 유추가 가능한 부분에 불필요한 주석은 제거하는 것이 나은 것 같습니다.
    • 자바에서는 Except라는 단어는 예민(?)하지 않을까 싶습니다. 대체어가 없을까요?
    • getPawnPieces() : char temp로 받고 ‘p’와 비교한 다음, append하면 될 것 같습니다.
  • Piece
    • enum이름은 빼도될 것 같습니다.
    • enum 요소는 그냥 개행해주는게 보기 좋을 것 같습니다.
    • create는 그냥 return에 한줄로 작성 가능할 것 같습니다.
  • Main
    • try~with~resource가 나은 것 같습니다.
    • Board는 그냥 start할때만 생성하는 것이 안전해 보입니다.
    • System.exit(0)이 있는데 break는 의미가 없어보입니다.
  • StringUtils
    • append와 +는 현재는 별 차이 없는 것 같습니다.

[Wan]

  • assertEquals -> assertThat
  • 32 -> 상수 필드로 선언해두는 것이 가독성이 좋을 것 같다.
  • Board가 while문 안에서 계속 생성되는 것 같다.
  • StringUtils에서 상수 필드 접근제어자를 private으로 변경하는 것이 나은 것 같다.

[roach]

  • Board에 blankDot을 final로 상수처리하는 것이 좋은 것 같다.
  • getExceptPawnPieces에서 그냥 String으로 변경하지 않아도 될 것 같다.
  • Piece에 create 시리즈에서 중복 코드를 좀 더 줄일 수 있을것같다.
  • StringUtils에서 StringBuilder가 필요없어 보인다. 단지 하나를 append하는 경우에는 그냥 +연산 하는것이 나을 것 같다. 연속적인 append같은 단순한 연산은 그냥 +가 나은 것 같다.

[woody]

  • Piece의 생성자가 추가되어서 create메서드의 코드를 줄일 수 있다.
  • enum네이밍에서 enum을 빼면될 것 같다.
  • 메서드 이름을 명확히하면 주석이 필요없을 것 같다.
  • honux의 chess는 헷갈리므로 빼는 게 좋을 것 같다.
  • 중복이 많아서 클래스나 메서드 분리를 하는 것이 좋은 것 같다. (디미터의 법칙)
  • Main의 default에 “메시지를 입력하세요”는 에러인 것 같지가 않다.

[프레디]

  • String의 +연산은 초기화에서는 그냥 이어붙인것과 같아서 연산을 수행하지 않는다. 따라서 바로 + 해서 리턴한다면 그냥 가독성이 좋은 방향으로 작성하는 것이 좋다.
  • 상수와 인스턴스의 이해가 추가적으로 필요해 보인다.
  • 생성자는 상단에 위치하는 것이 좋다.
  • enum 네이밍은 Piece랑 겹치는 것이 잘못되었다는 의미
  • 코드 정렬이 전반적으로 안되었다.
  • 패키지 생성도 잘못된 위치에 생성된 것 같다.
  • PR을 머지전에 본인이 닫으면 안된다.

스프링은 자유를 제한함으로써 코드를 획일화 하는 효과가 있다.


리뷰에서 나온 주제

문자열 + 연산과 StringBuilder

JLS 15.18.1에 따르면 문자열 + 연산의 결과는 새로 만든 String 객체지만, 식이 상수 식(constant expression)이면 예외다. 리터럴과 상수끼리 이어 붙인 식은 컴파일 시점에 하나의 문자열로 계산되므로 실행 중에 연결 연산이 일어나지 않는다. 상수가 아닌 값을 이어 붙일 때도 명세는 컴파일러가 StringBuffer나 비슷한 기법으로 중간 String 객체를 줄일 수 있다고 허용한다. 그래서 한 번 이어 붙여 바로 반환하는 정도라면 StringBuilder를 직접 쓰기보다 읽기 쉬운 +가 낫다는 리뷰가 나왔다.


try-with-resources

Java 튜토리얼에서 try-with-resources는 하나 이상의 자원을 선언하는 try 문이고, 문이 끝날 때 각 자원이 닫히도록 보장한다. AutoCloseable을 구현한 객체는 모두 자원으로 쓸 수 있다.


assertEquals 대신 assertThat

AssertJ는 많은 종류의 단언과 도움이 되는 실패 메시지를 제공하고 테스트 코드의 가독성을 높이는 것을 목표로 하는 라이브러리다. assertThat(actual).isEqualTo(expected)처럼 검사할 값에 단언 메서드를 이어 붙이는 형태다.


디미터의 법칙

디미터의 법칙(최소 지식 원칙)은 한 단위가 가까운 단위에 대해서만 제한적으로 알고, 친구하고만 이야기하고 낯선 대상과는 이야기하지 말라는 설계 지침이다. 위키백과는 이를 느슨한 결합의 특수한 경우로 설명한다. woody의 리뷰가 클래스나 메서드 분리를 이야기하며 이 원칙을 언급했다.

참고한 자료외부 출처 8

외부 출처

이 글은 저작권자의 CC BY 4.0 라이선스를 따릅니다.

댓글

아직 댓글이 없습니다