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

#109 댓글 닉네임 구현하기 #121

Merged
merged 10 commits into from
Aug 23, 2020
Merged

Conversation

hwanghe159
Copy link
Collaborator

@hwanghe159 hwanghe159 commented Aug 22, 2020

resolved #109

지금은 일단
작성자는 닉네임이 "작성자"로,
작성자 아닌 사람은 "익명#273", "익명#936" 이렇게 랜덤(#1 ~ #1000)으로 부여됩니다.
(당연히 같은 사람이 여러번 댓글달면 닉네임은 모두 같구요!)

나중에 닉네임이 새로 생성되는 방법을 변경시키려면
NicknameGenerator.createNewNickname() 메서드만 변경하면 됩니다.

- 댓글생성요청DTO에서 memberId, nickname 제거
- 본인의 글일 경우 닉네임은 '작성자'
- 이전에 댓글 단 적이 있으면 그 닉네임 가져오기
- 이전에 댓글 단 적이 없으면 새로 생성
- 댓글생성요청DTO에서 memberId, nickname 제거
- 본인의 글일 경우 닉네임은 '작성자'
- 이전에 댓글 단 적이 있으면 그 닉네임 가져오기
- 이전에 댓글 단 적이 없으면 새로 생성
Copy link
Collaborator

@include42 include42 left a comment

Choose a reason for hiding this comment

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

안녕하세요 카프카입니다.

와...정말 보면서 감탄했네요.
사실 도메인이 많이 바뀔 줄 알고 걱정했던 포인트였는데,
Article에 닉네임 로직을 조금 추가하고 Generator를 만들어서 다 해결한게
정말 좋은 방법이었다고 생각해요!! 확장성과 테스트는 덤...

프로젝트를 하면서 참 많이 배워가는구나 감사할 때가 많아요.
이 PR은 바로 approve할게요!! 고생하셨습니다.

@@ -25,6 +27,7 @@ public Comment createComment(Member member, CommentCreateRequest commentCreateRe
Comment comment = commentCreateRequest.toComment();
comment.setArticle(article);
comment.setMember(member);
comment.setNickname(nicknameGenerator.generate(member, article));
Copy link
Collaborator

Choose a reason for hiding this comment

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

nickname을 별도 entity로 만든다던가 하는 복잡한 로직만 생각했는데...
NicknameGenerator에서 만들어준다는 발상이 너무 좋네요
리스펙트 👍 👍 👍 👍

import static com.saebyeok.saebyeok.util.NicknameGenerator.WRITER_NICKNAME;
import static org.assertj.core.api.Assertions.assertThat;

public class NicknameGeneratorTest {
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

Choose a reason for hiding this comment

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

닉네임 생성기까지 테스트를 구현해 주셨네요!

Copy link
Collaborator

@hotheadfactory hotheadfactory left a comment

Choose a reason for hiding this comment

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

그래 수고하셨어요~ 이제 점점 테스트로 박아 놓았던 요소들이 사라지고 새벽이 모양을 갖추는 느낌이에요 :)
Approve 드릴게요!

@@ -52,4 +54,17 @@ public void setMember(Member member) {
public boolean isWrittenBy(Member member) {
return this.member == member;
}

public Optional<String> loadExistingNickname(Member member) {
return comments.stream()
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서 해당 게시글의 멤버와 매핑된 닉네임들을 불러오는군요!

import static com.saebyeok.saebyeok.util.NicknameGenerator.WRITER_NICKNAME;
import static org.assertj.core.api.Assertions.assertThat;

public class NicknameGeneratorTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

닉네임 생성기까지 테스트를 구현해 주셨네요!

.orElse(createNewNickname(article));
}

private String createNewNickname(Article article) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

여길 보면서 한가지 아이디어가 떠올랐었는데,
'이 과정에서 Repositpry에 너무 많이 접근하게 되는 것 같은데, 자바의 랜덤 중 Random rand = new Random(seed);를 이용해 시드값에 Article ID와 Member ID를 Serialize해서 넣으면 랜덤 닉네임을 생성하면서도 같은 게시글에 같은 댓글 작성자라면 언제나 같은 닉네임이 나오지 않을까?'
라는 생각이었는데, 생각해보니 1000이라는 작은 숫자 범위 때문에 어차피 방어 로직이 필요하겠네요...

뒤탈 없는 구현으로는 이게 더 나은 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 이런 구현 방법도 있네요!
createNewNickname() 내부의 로직은 어차피 바뀔(?) 운명이라서 좀 대충짜긴 했어요..ㅎㅎ

Copy link
Collaborator

@chomily chomily left a comment

Choose a reason for hiding this comment

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

닉네임 생성을 어떻게 할 수 있을까하고 어렵다 생각했는데, 이렇게 간단하게 해결해버렸네요!
구현도 클래스들 사이 관계도 깔끔하고 너무 감동이에요 🙊
최고최고 👍

다만 최근에 머지된 develop 브랜치 내용이 반영 덜 된 부분이 있어서
PR 머지하기 전에 Rebase 한 번 해주세요!

Approve 할게요!

Comment on lines +21 to +25
private Member me;
private Member other;
private Article myArticle;
private Article othersArticle;

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 +20 to +26
public String generate(Member member, Article article) {
if (article.isWrittenBy(member)) {
return WRITER_NICKNAME;
}
return article.loadExistingNickname(member)
.orElse(createNewNickname(article));
}
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

@icyMojito icyMojito left a comment

Choose a reason for hiding this comment

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

그래 선배님 닉네임을 박살내 버리셨다 🙊
저도 막연히 와 복잡하겠다 싶었는데 NicknameGenerator 보고 감탄했읍니다,,
테스트에도 정성이 가득하네요 approve 올립니다 👍🏻 💯

@hwanghe159 hwanghe159 merged commit 9ebbe21 into develop Aug 23, 2020
@hwanghe159 hwanghe159 deleted the feature/109-comment-nickname branch August 23, 2020 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

댓글 닉네임 구현하기
5 participants