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

랜덤 이름 생성기 #157

Merged
merged 6 commits into from
Jul 24, 2024
Merged

랜덤 이름 생성기 #157

merged 6 commits into from
Jul 24, 2024

Conversation

Hwanvely
Copy link
Collaborator

관련 이슈

변경 사항

AS-IS

Apple 로그인 사용자 이름은 랜덤한 알파벳의 나열로 이름이 정해졌고 Google, Kakao 로그인 사용자 이름은 제공된 이름으로 정하였습니다.

TO-BE

게스트를 제외한 모든 사용자의 이름을 첫 생성시 고유어를 이용한 부사_형용사_스낵이의 형태로 제공합니다.


😰 추가사항

테스트 결과 영어는 13자를 넘으면 레벨이 밀리는 문제가 발생하고 한글은 7가자 넘으면 두줄로 표현되게 됩니다...

@Hwanvely Hwanvely added the feat label Jul 23, 2024
@Hwanvely Hwanvely requested a review from 0chil July 23, 2024 13:24
@Hwanvely Hwanvely self-assigned this Jul 23, 2024
Copy link
Collaborator

@0chil 0chil 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 +10 to +15
private val socialMemberNameRandomizer = SocialMemberNameRandomizer()

@Test
fun `이름은_'_스낵이'로 끝나야_한다`() {
val randomized = socialMemberNameRandomizer.getName().string

Copy link
Collaborator

Choose a reason for hiding this comment

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

꿀팁! 하나 드립니다!
`이 안에서는 언더바를 안써도 됩니다`

Copy link
Collaborator Author

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 34
fun getRandomizedPureWord(): String {
val adjective = adjectives[(adjectives.indices).random()]
val determiner = determiners[(determiners.indices).random()]

return determiner + "_" + adjective
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

얘도 외부에서 쓸 일 없기 때문에 private으로 닫아 주시면 어떨까요?
+ 이 친구는 companion object에 들어가있지 않아도 될 것 같습니다!

Comment on lines 27 to 30
SocialMember member = members.findByProviderAndProvidedId(attributes.getProvider(), attributes.getId())
.orElseGet(() -> new SocialMember(
distinctNaming.from(new Name(attributes.getName())),
distinctNaming.from(nameRandomizer.getName()),
new ProfileImage(attributes.getPictureUrl()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

오.. 훨씬 깔끔해졌군요!

@@ -3,6 +3,6 @@
public class NameLengthException extends MemberException {

public NameLengthException() {
super("이름은 2글자 이상이어야 합니다");
super("이름은 2글자 이상 16글자 이하여야 합니다");
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 -19 to 21
private val nameRandomizer: NameRandomizer,
private val socialMemberNameRandomizer: SocialMemberNameRandomizer,
private val idTokenResolver: IdTokenResolver
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

나중에 MemberNameRandomizer라고 이름을 변경해서 좀 더 넓은 사용 범위로 표현해도 좋을 것 같네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 생각입니다 ㅎㅎ

Comment on lines 29 to 30
val adjective = adjectives[(adjectives.indices).random()]
val determiner = determiners[(determiners.indices).random()]
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 indices는 첨보는 친군데 이것도 깔끔하네요. index 배열을 주는 친구인가요?
저는 adjectives.random().take(1) 이런걸 생각했었습니다 😂

이 편이 더 효율적이긴 하겠네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 해당 array 및 collection의 유효한 index 범위를 반환해준다고 합니다!
저도 이번에 알았어요 👍

@Hwanvely Hwanvely requested a review from 0chil July 24, 2024 07:36
Copy link
Collaborator

@0chil 0chil left a comment

Choose a reason for hiding this comment

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

질문 1개, 컨벤션 제안 1개 드렸습니다

Comment on lines +18 to 20
@Size(min = 2, max = 13, message = "이름은 2글자 이상 13글자 이하여야 합니다")
private String name;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여긴 13글자 이하(13글자 가능)인데

@@ -34,7 +34,7 @@ private void validateNotNull(String string) {
}

private void validateLengthOf(String string) {
if (string.length() < 2 || string.length() > 16) {
if (string.length() < 2 || string.length() > 13) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여긴 13글자 미만(13글자 불가능) 이군요.
뭐가 맞는건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

13글자 초과 시 예외발생이니 13글자 이하가 가능한 것 아닌가요?? 😵‍💫

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 29 to 35

private fun getRandomizedPureWord(): String {
val adjective = adjectives[(adjectives.indices).random()]
val determiner = determiners[(determiners.indices).random()]

return determiner + "_" + adjective
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kotlin 코드 컨벤션#클래스 레이아웃에서 companion object를 가장 하단에 위치하도록 얘기하고 있습니다.

The contents of a class should go in the following order:

Property declarations and initializer blocks

Secondary constructors

Method declarations

Companion object

아무래도 상수같은 보조적 성격이 강하다보니 그런거라고 추측하는데요,
우리도 코틀린 기본 컨벤션을 사용하고 있기 때문에 반영해주면 좋겠네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이런게 있었군요..! 오늘도 또하나 배워갑니다 🙏

@Hwanvely Hwanvely requested a review from 0chil July 24, 2024 07:52
Comment on lines +17 to +20
}


}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

정해둔 배열에 존재하는 단어만을 가지고 생성되는 것을 검증하고 싶은데 방법이 있을까요?🥲

Copy link
Collaborator

@0chil 0chil Jul 24, 2024

Choose a reason for hiding this comment

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

adjectives, determiners를 외부에 열면 되겠습니다.

image
열어도 큰 문제는 없어보이네용

Comment on lines +10 to +15
private val socialMemberNameRandomizer = SocialMemberNameRandomizer()

@Test
fun `이름은_'_스낵이'로 끝나야_한다`() {
val randomized = socialMemberNameRandomizer.getName().string

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 이런 편한게 있었다니..!

Comment on lines 29 to 30
val adjective = adjectives[(adjectives.indices).random()]
val determiner = determiners[(determiners.indices).random()]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 해당 array 및 collection의 유효한 index 범위를 반환해준다고 합니다!
저도 이번에 알았어요 👍

Comment on lines -19 to 21
private val nameRandomizer: NameRandomizer,
private val socialMemberNameRandomizer: SocialMemberNameRandomizer,
private val idTokenResolver: IdTokenResolver
) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 생각입니다 ㅎㅎ

Comment on lines 29 to 35

private fun getRandomizedPureWord(): String {
val adjective = adjectives[(adjectives.indices).random()]
val determiner = determiners[(determiners.indices).random()]

return determiner + "_" + adjective
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이런게 있었군요..! 오늘도 또하나 배워갑니다 🙏

@@ -34,7 +34,7 @@ private void validateNotNull(String string) {
}

private void validateLengthOf(String string) {
if (string.length() < 2 || string.length() > 16) {
if (string.length() < 2 || string.length() > 13) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

13글자 초과 시 예외발생이니 13글자 이하가 가능한 것 아닌가요?? 😵‍💫

@Hwanvely Hwanvely merged commit 119b171 into dev Jul 24, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

사용자의 이름을 랜덤 생성한다 V2
2 participants