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

프로덕션 코드 리팩토링 #206

Merged
merged 18 commits into from
Dec 19, 2023
Merged

프로덕션 코드 리팩토링 #206

merged 18 commits into from
Dec 19, 2023

Conversation

yumyeonghan
Copy link
Collaborator

이슈번호

close: #205

작업 내용 설명

  • String 문자열 상수 처리
  • dto 컨벤션 통일
  • 데이터 타입 통일
  • 테스트 픽스처 정리

리뷰어에게 한마디

yumyeonghan added 8 commits December 18, 2023 15:33
@yumyeonghan yumyeonghan added the refactor 리팩터링 label Dec 18, 2023
@yumyeonghan yumyeonghan added this to the 7차 스프린트 milestone Dec 18, 2023
@yumyeonghan yumyeonghan self-assigned this Dec 18, 2023
Copy link

github-actions bot commented Dec 18, 2023

Test Results

  84 files    84 suites   16s ⏱️
261 tests 261 ✔️ 0 💤 0
262 runs  262 ✔️ 0 💤 0

Results for commit 25dd8e1.

♻️ This comment has been updated with latest results.

@@ -128,7 +127,7 @@ public ResponseEntity<Void> dismissReport(
}

@GetMapping("/reports")
public ResponseEntity<AdminCustomSlice<ReportListHTTP.Response>> findAllReports(
public ResponseEntity<AdminCustomSlice<ReportDto>> findAllReports(
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 왜 Dto 인가여 HTTP.Response

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AdminCustomSlice는 HTTP 대신 Slice로 반환하고 있습니다.

public record AdminCustomSlice<T>(
    List<T> contents,
    boolean hasNext
)

단일 dto는
Dto -> HTTP 로
복수 dto는
Dto 여러개가 -> Slice로
반환 되기 때문에 통일성을 주고자 이렇게 했습니다.

@@ -2,7 +2,7 @@

import coffeemeet.server.certification.domain.Certification;

public record PendingCertification(
public record PendingCertificationDto(
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 Dto을 제거하는게 맞아보는데요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이것도 위에서 커멘트에서 작성한 것과 같이 같은 의미로 통일성을 주고자 dto를 붙였습니다.

@@ -3,7 +3,11 @@
import org.springframework.data.domain.Page;

public record PendingCertificationPageDto(
Copy link
Collaborator

@smartandhandsome smartandhandsome Dec 19, 2023

Choose a reason for hiding this comment

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

얘가 Dto이기 떄문에

Page 안에 값은 Dto보단 PendingCertification인 VO로 그대로 두는게 적절해 보입니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

전달 객체는 PendingCertificationPageDto 이고 PendingCertificationDto는 전달 객체가 아니기 때문에 PendingCertification로 하는게 적절해 보입니다

Copy link
Collaborator Author

@yumyeonghan yumyeonghan Dec 19, 2023

Choose a reason for hiding this comment

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

PendingCertificationPageDto
PendingCertificationDto
이거 둘다 직접/간접적으로 전달되는거라
서비스에서 컨트롤러로 전달되는 객체에 Dto라는 네이밍을 쓰기로했으면 통일? 하는것도 나쁘지 않아보이는데 어떻게 생각하시나요

@@ -48,7 +49,7 @@ void generateTest() {
REFRESH_TOKEN);

// when
AuthTokens authTokens = authTokensGenerator.generate((long) Math.random());
AuthTokens authTokens = authTokensGenerator.generate(Faker.instance().random().nextLong());
Copy link
Collaborator

Choose a reason for hiding this comment

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

instancio를 쓰는게 어떨까요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영했습니다.!

@@ -65,7 +66,8 @@ void reissueAccessTokenTest() {
REFRESH_TOKEN);

// when
AuthTokens authTokens = authTokensGenerator.reissueAccessToken((long) Math.random(),
AuthTokens authTokens = authTokensGenerator.reissueAccessToken(
Faker.instance().random().nextLong(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

instancio를 쓰는게 어떨까요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영했습니다.!

@yumyeonghan yumyeonghan merged commit f621cab into dev Dec 19, 2023
3 checks passed
@yumyeonghan yumyeonghan deleted the refactor/#205 branch December 19, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor 리팩터링
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🦾 [Refactor] 프로덕션 코드 리팩토링
2 participants