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

Hongseok week1 #3

Open
wants to merge 6 commits into
base: hongseok
Choose a base branch
from
Open

Hongseok week1 #3

wants to merge 6 commits into from

Conversation

bayy1216
Copy link
Collaborator

작업내용

  • entity, repository 코드 작성
  • 게시글 생성 기능 1개 구현

@bayy1216 bayy1216 self-assigned this Mar 23, 2024

@Configuration
@EnableJpaAuditing
public class JpaConfig {
Copy link
Collaborator

Choose a reason for hiding this comment

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

깡통 JpaConfig에 @EnableJpaAuditing을 올렸다는게 인상


@Transactional
public Long createArticle(Long id, ArticleCreateRequest articleCreateRequest) {
MemberEntity member = memberJpaRepository.findById(id).orElseThrow(() -> new ResourceNotFoundException("Member", id));
Copy link
Member

Choose a reason for hiding this comment

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

저라면 여기 예외처리 부분을 repository로 뺄 꺼 같긴 합니다 !
article을 만드는 것과는 분리되는 작업이라서 ..?

Copy link
Member

Choose a reason for hiding this comment

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

말씀하신대로 레포지토리가 DB엑세스만 담당하니까 비지니스 로직인 예외처리를 서비스 레이어에서 해야하는 것도 맞는 거 같습니다

Copy link
Member

Choose a reason for hiding this comment

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

근데 저의 개인적인 의견은
동일한 코드가 중복되어 작성될 가능성이 있고
member단일에 대한 SQL만으로도 검증이 가능한 코드이기 때문에 레포지토리 레이어에서도 해줄 수 있다고 생각했습니다

Copy link
Member

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.

다시 생각해보니 맞는의견같아서 지웠어요.. ㅎㅎ
repository에서 쓰는게 좋다고 생각했습니다!

근데 이게 코틀린처럼 확장함수를 못쓰고, 인터페이스로하려니까 꽤나 귀찮은거같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

메소드명은 findByIdOrThrow로 하면 깔끔할거 같습니다!

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.

3 participants