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.

Sign up for GitHub

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

Jump to bottom

Step1: 리팩터링 #969

Open
wants to merge 10 commits into
base: hungryjayy
Choose a base branch
from
Open

Step1: 리팩터링 #969

wants to merge 10 commits into from

Conversation

hungryjayy
Copy link

  • TDD + 리팩터링 위주의 작업을 한 PR입니다.!
  • Question의 List를 일급 콜렉션으로 구현하는 힌트가 있는데, 현재 List이 쓰이는 곳이 없는 것으로 보여 이 것 대신에 List 를 일급 컬렉션으로 구현해보았습니다.

vagabond95
vagabond95 requested changes Nov 14, 2022
View reviewed changes
Copy link

@vagabond95 vagabond95 left a comment

Choose a reason for hiding this comment

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

안녕하세요. 볼링 미션 진행동안 잘 부탁드려요 :)

객체가 스스로 일을 처리하는 관점에서 고민해볼 포인트가 있어 리뷰 남겼어요. 확인 부탁드립니다!

src/main/java/qna/domain/DeleteHistories.java
this.deleteHistories = deleteHistories;
}

public void add(long questionId, Question question) {
Copy link

Choose a reason for hiding this comment

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

DeleteHistory 내부에서 question 과 answer 을 다루기보다
anwser 과 question 이 DeleteHistory 를 리턴하는 구조가 더 자연스럽지 않을까요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vagabond95 vagabond95 vagabond95 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hungryjayy @vagabond95