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

[CHORE] 특정 값 변수 모음 클래스 병합 #176

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

thguss
Copy link
Member

@thguss thguss commented Jan 29, 2024

✨ Related Issue

📝 기능 구현 명세

none

🐥 추가적인 언급 사항

  • Entity 내에서 사용되는 final 값은 의존성을 주입할 수 없어 static으로 분류했습니다.
  • .replace(get~(), get~()) 코드가 반복되는데, 짧지 않은 길이어서 공통 메서드로 묶었습니다. 이전 코드가 더 좋다고 생각되면 되돌려놓겠습니다!
  • 잘못된 점이나 더 좋은 방향이 있다면 리뷰 부탁드려요 :)

@thguss thguss added sohyeon 소현 작업 fix labels Jan 29, 2024
@thguss thguss self-assigned this Jan 29, 2024
Copy link
Contributor

@Chan531 Chan531 left a comment

Choose a reason for hiding this comment

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

너무나도 빠르다....

.orElseThrow(() -> new MemberException(INVALID_MEMBER));
val token = generateAccessToken(new UserAuthentication(member.getId(), null, null));
return TokenResponse.of(token);
}

private String getBearerToken(String token) {
Copy link
Contributor

Choose a reason for hiding this comment

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

메서드 뺀 거 좋은 거 같아요~
근데 "Bearer "를 뺀 나머지 string값을 받아오는 거라 메서드 이름을 바꿔야할 거 같긴한데... 어떻게 생각하시나요??

Copy link
Member Author

Choose a reason for hiding this comment

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

지피티한테 자문 한 번 구해보겠습니당!!
그 외에 추천해주실 메소드명 있을까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

@Chan531 우선 "getTokenFromBearerString"으로 메소드 변경해서 병합해두겠습니다!
더 좋은 메소드명이 있다면 언제든 공유 부탁드려요~!

Copy link
Contributor

@csb9427 csb9427 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~~

Comment on lines +98 to +99
val kid = ((JsonObject) JsonParser.parseString(header)).get(valueConfig.getKID_HEADER_KEY());
val alg = ((JsonObject) JsonParser.parseString(header)).get(valueConfig.getALG_HEADER_KEY());
Copy link
Contributor

Choose a reason for hiding this comment

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

뭔가 더 좋은 걸로 결론은 낫지만 코드가 길어진 거 같아 슬프긴 하군요..

Copy link
Member Author

Choose a reason for hiding this comment

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

ㅠ.ㅠ 계속 더 좋은 방향으로 찾아봅시당,,

@thguss thguss merged commit 9f83cd2 into develop Jan 30, 2024
1 check passed
@thguss thguss deleted the chore/#173-merge-valueConfig branch January 30, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix sohyeon 소현 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CHORE] Constant, ValueConfig 병합 제안
3 participants