Skip to content

feat : feed 도메인 리팩토링#238

Merged
jun23314 merged 19 commits intodevelopfrom
refactor/#220_feed
May 9, 2025
Merged

feat : feed 도메인 리팩토링#238
jun23314 merged 19 commits intodevelopfrom
refactor/#220_feed

Conversation

@Seungcode
Copy link
Collaborator

#️⃣연관된 이슈

📝작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요
feed 도메인 리팩토링 및 v2 생성

💬리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

@Seungcode Seungcode requested a review from jun23314 May 9, 2025 10:49
@Seungcode Seungcode self-assigned this May 9, 2025
@Seungcode Seungcode added the 🔧 Refactor 기능 고도화, 리팩토링을 진행할 때 사용합니다 label May 9, 2025
@Seungcode Seungcode linked an issue May 9, 2025 that may be closed by this pull request
2 tasks
@github-actions
Copy link

github-actions bot commented May 9, 2025

Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit acb11f3. ± Comparison against base commit 377d730.

♻️ This comment has been updated with latest results.

@@ -11,10 +11,10 @@ public record FoodResponse(
) {

public static FoodResponse toResponse(Food food) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

of로 통일하기로 했어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

v1 코드는 안고치는거 아니었나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

쏘리

return FoodsResponse.builder()
.foods(foodResponses)
.build();
return new FoodsResponse(foodResponses);
Copy link
Collaborator

Choose a reason for hiding this comment

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

팩토리 메서드(of)로 사용하는 데, new를 사용하는 것은 잘못된 방식인 것 같아요.(record 타입 기본 생성자 private으로 변환 필요해 보임. 모든 인원이 해당 되며, @NoArgsConstructor 활용하면 될 듯 합니다.)

Copy link
Contributor

Choose a reason for hiding this comment

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

이것도 v1인데 고쳐야 하나요..?

Copy link
Collaborator

Choose a reason for hiding this comment

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

쏘리

Comment on lines +66 to +70
private void checkAmount(long amount, long point){
if (amount < point) {
throw new PointNotEnoughExceptionV2();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳이 이 로직만 따로 함수화한 이유가 있나요?

Comment on lines +28 to +32
public ResponseEntity<ApiResponse<Void>> payFoodWithPoints(@LoginUser LoginInfo loginInfo,
@PathVariable Long foodId,
@RequestBody DeliveryFeeRequest request) {
foodServiceV2.processPointPaymentForFood(loginInfo.memberId(), foodId, request.deliveryFee());

Copy link
Collaborator

Choose a reason for hiding this comment

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

controller랑 service랑 로직이 1대1로서 같은 의미인데, 메서드 명이 다른 것이 확인하기 힘든 것 같은데,,, 다들 어떻게 생각해요?

Copy link
Contributor

Choose a reason for hiding this comment

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

이건 저도 하나로 통일하는게 좋을 것 같아요

@jun23314 jun23314 merged commit afd3c7c into develop May 9, 2025
2 of 3 checks passed
@jun23314 jun23314 deleted the refactor/#220_feed branch May 9, 2025 11:40
2dhhh pushed a commit that referenced this pull request May 9, 2025
2dhhh pushed a commit that referenced this pull request May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔧 Refactor 기능 고도화, 리팩토링을 진행할 때 사용합니다

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REFACTOR] 사료 도메인 리팩토링

3 participants