refactor : file exist add #305
Conversation
Eseas
requested review from
HiSeungmin,
K-KY,
jeongns2611,
kdh10806 and
taekminKwon
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| this.fileExists = fileExists; | ||
| } | ||
|
|
||
| public static PostListResponse toDto(Post post, Long commentCount) { |
There was a problem hiding this comment.
코드 리뷰를 진행하겠습니다. 이 패치에서 몇 가지 중요한 사항과 개선할 점이 있습니다.
-
변수 이름의 일관성:
fileExists변수는 불리언 값을 나타내며 존재 여부를 나타내는 데 사용됩니다. 그러나 이 변수의 Getter 메소드가 없기 때문에 외부에서 이 정보에 접근할 수 없습니다. Getter 메소드를 추가하는 것이 좋습니다. -
Boolean과 boolean의 사용:
fileExists는Boolean타입으로 정의되어 있습니다. 이는 null 값도 가질 수 있지만, 존재 여부만 체크하는 경우boolean타입으로 사용하는 것이 더 안전하며, 불필요한 null 체크를 피할 수 있습니다. -
기본값 설정: 생성자에서
this.fileExists를false로 초기화하는 것은 좋은 접근입니다. 그러나 이는 해당 정보가 항상 미리 설정된다고 가정하고 있기 때문에, 상황에 따라 상황에 맞는 값으로 초기화할 필요성이 있을 수 있습니다. 생성자를 통해 파일 존재 여부를 매개변수로 받아 초기화하는 것도 고려해 볼 수 있습니다. -
주석 추가: 존재 여부를 뜻하는
fileExists필드에 대한 설명이나 필요한 처리를 주석으로 추가하는 것이 코드의 가독성을 높이는 데 도움이 될 것입니다. -
메소드 이름 수정 제안:
setFileExist메소드는setFileExists로 이름을 변경하는 것이 좋습니다. 메소드 이름과 변수 이름의 일관성을 유지하면 코드의 이해도를 높일 수 있습니다. -
정상 작동 여부 검증: fileExists의 값을 설정할 때, valid 상태(예: 파일이 실제로 존재하는지)의 체크가 필요한 경우가 있을 수 있습니다. 적절한 예외 처리나 검증 로직을 추가하는 것이 좋습니다.
이러한 점들을 고려해 보시고, 코드의 안정성과 가독성을 높이는 방안에 대해 한 번 더 점검해 보시기 바랍니다.
| ); | ||
|
|
||
| return PaginatedResponse.createPaginatedResponse(repoPostList); | ||
| } |
There was a problem hiding this comment.
코드 리뷰 결과는 다음과 같습니다:
-
버그 위험:
postFileService.getPostFiles(post.getPostId())메소드를 호출하여 파일의 존재 여부를 확인하는 부분에서, 만약 이 메소드가 예외를 발생시킬 경우 해당 예외 처리가 없습니다. 이로 인해 프로그램이 중단될 수 있습니다. 예외 처리를 추가하는 것이 좋습니다.
-
성능 차원:
postFileService.getPostFiles(post.getPostId())호출이 여러 번 이루어지면, 성능 문제가 발생할 수 있습니다. 각 게시물의 파일 존재 여부를 확인하기 위해 여러 번 데이터베이스를 호출하는 대신, 한번의 호출로 모든 게시물에 대한 파일 존재 여부를 확인한 후, 결과를 캐싱하여 사용하도록 개선할 수 있습니다.
-
가독성:
forEach를 사용하는 대신,map을 사용하는 방법도 고려할 수 있습니다. 이러한 방식은 더 선언적인 코드 스타일을 제공하여 이해하기 쉽습니다. 다만, 이 경우 결과를 새로운 리스트로 반환하므로 이 로직이 필요한지 판단해야 합니다.
-
불필요한 호출:
PageRequest.of(page, PAGE_SIZE, getSort(isAllStages, sortType))부분에서 이미 페이지를 요청한 상태에서postFileService.getPostFiles(post.getPostId())를 부르는 상황이기 때문에, 페이지네이션이 필요한 경우 꼭 필요한 데이터만 요청하도록 리팩토링할 필요가 있습니다.
종합적으로 이러한 사항들을 고려하여 코드를 개선하면, 더 나은 성능과 안정성을 기대할 수 있을 것입니다.
바로 MERGE하지 마세요! 검토 후 MERGE해야합니다!
✅ 최근 작업 주제 (하나 이상의 주제를 선택해주세요.)
🏆 구현 목표
ex)
bucket post 및 bucket list view 초기 구현
📋 구현 사항 설명 (작업한 내용을 상세하게 기록합니다.)
ex)
🔍 테스트 방법 (변경 사항을 확인하기 위한 테스트 방법을 기술합니다.)
ex)
npm test실행🛠️ 쓰이는 모듈
ex)
💬 기타 질문 및 특이 사항
ex) 백프 배포 nginx 검토