피드백을 주고받은 PR
피드백 정리
1. for-loop를 Stream.forEach()로 바꾸지 말아야 할 이유
스트림은 Java8버전부터 나온 최신 기술입니다. 하지만 자바의 최신 기술이라고해서 무조건 좋은 성능을 갖는 것은 아닙니다. 저는 우리가 프로그래밍을 처음 배울때 사용하였던 for-loop와 stream의 for-each를 생각하였을 때, for-each가 더 최신 기술이라 더 좋은 성능을 가질 것이라 생각했습니다. 하지만 forEach는 for-loop보다 primitive타입에서 사용시 성능이 현저히 낮으며 iteration cost와 functionality cost의 합이 충분히 클 때, 비로소 성능이 비슷해집니다.
- 최신 기술이라고 모든 면에서 좋은건 아니다.
- stream의 forEach는 대체로 for-loop보다 성능이 떨어지며, iteration cost와 functionality cost의 합이 클 때 for-loop와 비슷한 성능을 가진다.
실무에서의
stream().forEach()는 개발자의 스타일에 따라 다르겠지만 리뷰어의 경험상 잘 사용되지 않는 것 같다고 합니다.
학습 자료:
2. util의 속성
📌 utils에는 비즈니스 로직과 직접적인 관계가 없고 범용적으로 사용할 수 있는 도구들이 있어야합니다.
해당 내용의 구체적인 정리는 utils의 이해 & 제너릭 인터페이스를 활용한 예외처리 에서 확인해보실 수 있습니다.
3. 숫자의 단위가 높아지는 경우 _를 사용하자
public enum Rank {
FIRST(2000000000, 6, false),
SECOND(30000000, 5, true),
THIRD(1500000, 5, false),
FOURTH(50000, 4, false),
FIFTH(5000, 3, false),
FAIL(0, 0, false);
}프로그래밍을 하며 숫자의 단위가 커지는 경우 가독성도 좋지 않고 실수를 유발할 수도 있습니다. Java7버전 이후부터는 숫자 Literal사이에는 원하는 수 만큼 Underscore(_)를 넣을 수 있다고 합니다. 이를 활용하면 코드를 더욱 가독성있게 변경할 수 있습니다.
public enum Rank {
FIRST(2_000_000_000, 6, false),
SECOND(30_000_000, 5, true),
THIRD(1_500_000, 5, false),
FOURTH(50_000, 4, false),
FIFTH(5_000, 3, false),
FAIL(0, 0, false);
}해당 내용의 구체적인 정리는 자바에서 큰 숫자를 가독성 좋게 하는 방법 에서 확인해보실 수 있습니다.
학습 자료
4. 파일의 마지막 줄 개행
IEEE가 책정한 POSTFIX 에서의 하나의 행을 정의하는 표준 때문에 파일의 끝에는 개행을 해야합니다. C 컴파일러인 gcc는 POSTFIX에 근거해서 동작하며 소스코드를 한줄씩(line by line)으로 읽습니다. 그리하여 파일이 끝났더라도 개행문자가 없으면 한 줄이 끝나지 않은 것으로 인식해서 정상적으로 동작하지 않는 문제가 발생할 수 있습니다.
프로그램을 실행하며 오류 없이 잘 수행될 수 있으나 잠재적인 오류가 발생할 수 있기에 파일의 끝에는 개행을 붙여줘야합니다.
해당 내용의 구체적인 정리는 No newline at a end of file, 파일의 끝에는 개행 추가 에서 확인해보실 수 있습니다.
5. 컨트롤러 내의 인스턴스
미션을 진행하며 다음과 같은 리뷰를 받았습니다... 1단계 미션을 진행하며 MVC에 대해 어느정도 학습을 하였다고 생각했지만 아직 부족했던 것 같습니다.
위의 피드백을 받고 곰곰히 생각해보니 컨트롤러가 인스턴스 변수를 갖고있게 된다면 컨트롤러와 모델의 관계가 연결된 것이 아니라, 컨트롤러 안에 Model이 들어가있는 형태가 만들어지게 됩니다.
위의 코드로 실행되는 형태를 그림으로 그리면 다음과 같습니다.
MVC 구현을 할 때, Controller는 Model과 View의 인터페이스 역할을 하여 model이나 view에 대해 알고 있을 수는 있으나 model이나 view의 인스턴스 변수를 생성하며 둘 사이의 의존성을 높이는 작업은 피하도록 하자!!!
6. IntelliJ의 추천 축약 기능을 사용하자
인텔리제이에서는 불필요한 코드가 들어간 경우 축약이 가능하다는 메시지로 코드를 형광펜 친 것과 같이 표현을 해줍니다.
이러한 경우 opt+enter를 통해 축약을 하여 불필요한 코드를 삭제하도록 합시다.
// 축약 전
lottos.purchase(new CustomPurchaseStrategy(Arrays.asList(new Integer[]{1, 2, 3, 43, 44, 45})));
// 축약 후
lottos.purchase(new CustomPurchaseStrategy(Arrays.asList(1, 2, 3, 43, 44, 45)));7. 테스트 클래스에 붙이는 @DisplayName은 피하자
미션을 진행하며 테스트 클래스 별로 @DisplayName을 클래스별로 각각 어떤 테스트 내용들이 있는지 알리고자 붙여봤습니다.
각각의 파일명에서 @DisplayName을 붙여두니 어떤 테스트들이 존재하는지 확실히 알 수 있어서 더 좋다고 생각하였습니다.
하지만 더 깊게 생각해보니 현재는 미션의 규모가 작아 테스트 오류가 발생하여도 어떤 파일에서 오류가 발생하였는지 쉽게 찾을 수 있는데
프로젝트 규모가 커져서 테스트 파일만 100개이상 존재하게 된다면 재정의한 @DisplayName으로
어떤 파일에서 테스트 실패가 나타나는지 확인하기 힘들어질 것입니다.
@DisplayName은 메서드 위에는 붙이더라도 클래스 자체에 붙이는 것은 지양하는 것이 좋을 것 같습니다.
8. 테스트 코드에도 변수들에 접근 제어자를 추가하자
9. final은 자신만의 기준을 정해서 사용하자.
개발을 하며 같은 상황임에도 불구하고 final을 붙이는 곳과 붙이지 않은 곳이 있는 등 코드의 일관성이 없었습니다.
그래서 리뷰어 코니는 이번 기회에 자신만의 final 사용 기준을 세워보라고 하여 저만의 사용 기준을 세워봤습니다.
- 클래스 변수: 가능하면 final을 붙이자.
- 인스턴스 변수: 생성자 내부에서 초기화 할 수 있으면 final을 붙이자.
- 메서드의 매개변수: 원시타입에는 final을 붙이고 그 외에는 붙이지 않는다.
- 메서드 내부의 변수: 재할당을 금지할 변수의 경우 final을 붙이자.
해당 기준을 세우고 리펙터링을 하며 메서드의 길이가 10줄도 안되는 이번 미션 코드의 경우,
오히려 final을 메서드의 매개변수와 변수에 붙이면 오히려 코드가 덜 깔끔해보이는??느낌이 들곤 하여 리뷰어의 final 사용 기준에 대해 질문을 남겼습니다.
리뷰어 코니의 답변은 다음가 같았습니다.
사실 이런 부분은 프로젝트 전체에 일관성이 있는 것이 가장 좋기 때문에, 기존 프로젝트의 방식이 있다면 그것을 따르는 것이 좋습니다. 하지만 일반적으로 클래스 변수와 인스턴스 변수에 붙이는 final은 중요한 의도를 품고 있기 때문에 때에 맞춰 사용하죠. 그 외에 파라미터나 로컬 변수의 경우엔 붙였을 때의 실익이 크지 않은 경우가 많아서 저는 사용하지 않는 편이에요.
개발자, 개발 팀에 따라 final의 사용 기준은 다르나 룰을 정하여 일관성만 있으면 된다고 합니다! 여러분도 이번 기회에 본인만의 final 사용 기준을 세워보는 것도 좋을 것 같아요😁
10. intelliJ의 초록줄을 무시하지 말아라
인텔리제이는 코드에 오타가 발생할 시 초록줄을 통해 개발자에게 알려줍니다.
이전까지 저는 이러한 초록줄을 무시하고 프로그래밍하였는데 이번 미션을 진행하며 많은 오타가 나왔고, 해당 오타에 대한 피드백들을 많이 받으며 앞으로는 IntelliJ가 주는 모든 피드백들을 무시하지 않고 유심히 보겠다는 다짐을 했습니다.
etc.
이번 미션을 진행하며 가장 많이 받았던 피드백들은 객체들의 분리가 잘 안되었다., 중복된 로직이 여러 클래스에 존재한다.,
네이밍이 좋지 않다., 로직들의 올바은 위치에 대한 피드백들이 80프로가 넘었던 것 같습니다.
해당 피드백들은 간략하게 코멘트 링크만 남기겠습니다.
