Skip to content
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.

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

[FEAT] 카테고리별 최신 게시물 조회 API 구현 - #434 #447

Merged
merged 7 commits into from
Nov 15, 2024

Conversation

rlarlgnszx
Copy link
Contributor

📝 PR Summary

🌴 Works

  • 카테고리별 최신 게시물 조회 API 구현

🌱 Related Issue

closed #434

🌵 PR 참고사항

  • Playground에 content값을 추가해서 요청했습니다
  • 아직 값이 없어 content는 null인 상태입니다.
    image

@rlarlgnszx rlarlgnszx added the ✨ Feat 새로운 피쳐 생성 label Nov 14, 2024
@rlarlgnszx rlarlgnszx self-assigned this Nov 14, 2024
Copy link

height bot commented Nov 14, 2024

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Member

@kseysh kseysh left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

public List<RecentPostsResponse> getRecentPosts(String playgroundToken) {
final Map<String, String> accessToken = createAuthorizationHeaderByUserPlaygroundToken(playgroundToken);
try (ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor()) {
List<String> categories = List.of("SOPT 활동", "자유", "파트");
Copy link
Member

Choose a reason for hiding this comment

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

P2: 카테고리는 ENUM으로 관리하는 것이 좋을 것 같은데 어떻게 생각하실까요?

Comment on lines 186 to 199
try (ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor()) {
List<String> categories = List.of("SOPT 활동", "자유", "파트");
CompletableFuture<RecentPostsResponse> hotPostFuture = CompletableFuture.supplyAsync(() ->
RecentPostsResponse.of(playgroundClient.getPlaygroundHotPost(accessToken)), executor);
List<CompletableFuture<RecentPostsResponse>> categoryFutures = categories.stream()
.map(category -> CompletableFuture.supplyAsync(() -> playgroundClient.getRecentPosts(accessToken, category), executor))
.toList();
List<CompletableFuture<RecentPostsResponse>> allFutures = new ArrayList<>(categoryFutures);
allFutures.addFirst(hotPostFuture);
CompletableFuture<Void> allOf = CompletableFuture.allOf(allFutures.toArray(new CompletableFuture[0]));
return allOf.thenApply(v -> allFutures.stream()
.map(CompletableFuture::join)
.collect(Collectors.toList()))
.join();
Copy link
Member

Choose a reason for hiding this comment

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

P2: 찾아보니 가상스레드로 병렬 실행을 하는 기능인 것 같아요! 처음 보는 기능이라 신기하네요!
try로 현재 묶어두고 이외의 예외처리는 하지 않았는데 그 이유가 궁금합니다

또한 stream의 병렬처리와는 어떤 다른 이점이 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 이건 try-catch 구조가 아니라
try with resource라는 개념의 try입니다!
excutor 객체를 할당하고 이후에 해제를 해야하는데 try resource 구문으로 블록에서 벗어 날 시에 소스 할당 해제를 하는 역할을 담당합니다!

또한 stream의 병렬처리와는 어떤 다른 이점이 있나요? -> 이건 생각못해봤는데 재밌는 질문입니다!
기본적으로 Stream에서는 List안의 데이터 대한 작업 처리를 병렬적으로 수행하는데 역할이 중점되어있다고 생각합니다.
비동기처리를 사용했을때는 작업시간,예외처리등의 방식을 좀더 유연하게 작성할수있다는 점에서 CompletableFuture를 사용했습니다.!

private String title;
private String category;
private String content;
private boolean isHotPost;
Copy link
Member

Choose a reason for hiding this comment

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

P1. primitive type은 Getter로 반환할 때 is라는 접두사가 빠지게 됩니다!
현재에도 반환값이 HotPost로 is 접두사가 빠져있는 것 같아요.
그리고 isHotPost라는 필드가 없더라도 클라이언트가 category로 구분이 가능할 것 같은데 어떻게 생각하시나요?
관련 블로그 글

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 이건 몰랐네요! 좋은 정보 감사합니다!
그러면 hot 쪽을 빼는걸로 할까요 ?
image
HotPost가 혹시 카테고리별로 작동될수 있다고 생각해서 일단 넣어두긴 했습니다!

Copy link
Member

Choose a reason for hiding this comment

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

hot post가 카테고리 별로 작용될 것 같아서 그런거라면 유지해도 좋을 것 같아요~

Comment on lines +20 to +27
public static RecentPostsResponse of(PlaygroundPostResponse playgroundPostResponse) {
return RecentPostsResponse.builder()
.id(playgroundPostResponse.postId())
.title(playgroundPostResponse.title())
.category("HOT")
.content(playgroundPostResponse.content())
.isHotPost(true)
.build();
Copy link
Member

Choose a reason for hiding this comment

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

P2: 그럼에도 만약 isHotPost를 사용하시는게 맞다 생각하면 이 메서드 명은 of보다는 팩토리 메서드처럼 객체를 만드는 메서드에 이름을 정해주는 것이 혼란을 방지하기에 좋을 것 같아요!

@rlarlgnszx rlarlgnszx merged commit f4130fb into dev Nov 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feat 새로운 피쳐 생성 size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 플레이그라운드 게시물 반환 기능
2 participants