Skip to content

refactor : file exist add #305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Eseas merged 1 commit into from
Feb 19, 2025
Merged

refactor : file exist add #305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Main/src/main/java/com/seveneleven/board/dto/PostListResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class PostListResponse {
private LocalDateTime createDate;
private LocalDate deadline;
private Long commentCount;
private Boolean fileExists;

private PostListResponse(Post post, Long commentCount) {
this.projectStepId = post.getProjectStep().getId();
Expand All @@ -35,6 +36,11 @@ private PostListResponse(Post post, Long commentCount) {
this.createDate = post.getCreatedAt();
this.deadline = post.getDeadline();
this.commentCount = commentCount;
this.fileExists = false;
}

public void setFileExist(boolean fileExists) {
this.fileExists = fileExists;
}

public static PostListResponse toDto(Post post, Long commentCount) {
Copy link

@github-actions github-actions bot Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 진행하겠습니다. 이 패치에서 몇 가지 중요한 사항과 개선할 점이 있습니다.

  1. 변수 이름의 일관성: fileExists 변수는 불리언 값을 나타내며 존재 여부를 나타내는 데 사용됩니다. 그러나 이 변수의 Getter 메소드가 없기 때문에 외부에서 이 정보에 접근할 수 없습니다. Getter 메소드를 추가하는 것이 좋습니다.

  2. Boolean과 boolean의 사용: fileExists는 Boolean 타입으로 정의되어 있습니다. 이는 null 값도 가질 수 있지만, 존재 여부만 체크하는 경우 boolean 타입으로 사용하는 것이 더 안전하며, 불필요한 null 체크를 피할 수 있습니다.

  3. 기본값 설정: 생성자에서 this.fileExists를 false로 초기화하는 것은 좋은 접근입니다. 그러나 이는 해당 정보가 항상 미리 설정된다고 가정하고 있기 때문에, 상황에 따라 상황에 맞는 값으로 초기화할 필요성이 있을 수 있습니다. 생성자를 통해 파일 존재 여부를 매개변수로 받아 초기화하는 것도 고려해 볼 수 있습니다.

  4. 주석 추가: 존재 여부를 뜻하는 fileExists 필드에 대한 설명이나 필요한 처리를 주석으로 추가하는 것이 코드의 가독성을 높이는 데 도움이 될 것입니다.

  5. 메소드 이름 수정 제안: setFileExist 메소드는 setFileExists로 이름을 변경하는 것이 좋습니다. 메소드 이름과 변수 이름의 일관성을 유지하면 코드의 이해도를 높일 수 있습니다.

  6. 정상 작동 여부 검증: fileExists의 값을 설정할 때, valid 상태(예: 파일이 실제로 존재하는지)의 체크가 필요한 경우가 있을 수 있습니다. 적절한 예외 처리나 검증 로직을 추가하는 것이 좋습니다.

이러한 점들을 고려해 보시고, 코드의 안정성과 가독성을 높이는 방안에 대해 한 번 더 점검해 보시기 바랍니다.

Expand Down
3 changes: 3 additions & 0 deletions Main/src/main/java/com/seveneleven/board/service/PostServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ public PaginatedResponse<PostListResponse> selectPosts(Boolean isAllStages, Long
} else {
repoPostList = postReader.getPostsByProjectStepId(projectStepId, keyword, repoFilter, PageRequest.of(page, PAGE_SIZE, getSort(isAllStages, sortType)));
}
repoPostList.getContent().forEach(post ->
post.setFileExist(!postFileService.getPostFiles(post.getPostId()).isEmpty())
);

return PaginatedResponse.createPaginatedResponse(repoPostList);
}
Copy link

@github-actions github-actions bot Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰 결과는 다음과 같습니다:

  1. 버그 위험:

    • postFileService.getPostFiles(post.getPostId()) 메소드를 호출하여 파일의 존재 여부를 확인하는 부분에서, 만약 이 메소드가 예외를 발생시킬 경우 해당 예외 처리가 없습니다. 이로 인해 프로그램이 중단될 수 있습니다. 예외 처리를 추가하는 것이 좋습니다.

  2. 성능 차원:

    • postFileService.getPostFiles(post.getPostId()) 호출이 여러 번 이루어지면, 성능 문제가 발생할 수 있습니다. 각 게시물의 파일 존재 여부를 확인하기 위해 여러 번 데이터베이스를 호출하는 대신, 한번의 호출로 모든 게시물에 대한 파일 존재 여부를 확인한 후, 결과를 캐싱하여 사용하도록 개선할 수 있습니다.

  3. 가독성:

    • forEach를 사용하는 대신, map을 사용하는 방법도 고려할 수 있습니다. 이러한 방식은 더 선언적인 코드 스타일을 제공하여 이해하기 쉽습니다. 다만, 이 경우 결과를 새로운 리스트로 반환하므로 이 로직이 필요한지 판단해야 합니다.

  4. 불필요한 호출:

    • PageRequest.of(page, PAGE_SIZE, getSort(isAllStages, sortType)) 부분에서 이미 페이지를 요청한 상태에서 postFileService.getPostFiles(post.getPostId())를 부르는 상황이기 때문에, 페이지네이션이 필요한 경우 꼭 필요한 데이터만 요청하도록 리팩토링할 필요가 있습니다.

종합적으로 이러한 사항들을 고려하여 코드를 개선하면, 더 나은 성능과 안정성을 기대할 수 있을 것입니다.

Expand Down
Loading