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

패키지 구조 리팩터링 및 세미나 게시물 저장 로직 리팩터링 #124

Merged
merged 3 commits into from
Feb 24, 2025

Conversation

kang20
Copy link
Member

@kang20 kang20 commented Feb 23, 2025

🌱 작업 사항

  • 회의 때 말한 사항대로 게시물 저장 로직만 리팩터링 해보았습니다.
  • Board 도메인 전부를 리팩터링 한게 아니니 회의 내용과 조금 다르다고 오해 주의 해주세요 BoardController에 createBoard 메서드부터 타고 보시면 됩니다
  • 작업 내용은 백엔드 기술문서 4번을 참고해주시고 질문은 slack을 통해 해주세요

@kang20 kang20 added the ♻️ Refactor 리팩토링 label Feb 23, 2025
@kang20 kang20 self-assigned this Feb 23, 2025
@kang20
Copy link
Member Author

kang20 commented Feb 23, 2025

📝 Jacoco Test Coverage

Overall Project NaN% NaN% 🍏

There is no coverage information present for the Files changed

Copy link

🧪 Test Results

37 tests  ±0   36 ✅ ±0   4s ⏱️ -1s
14 suites ±0    1 💤 ±0 
14 files   ±0    0 ❌ ±0 

Results for commit d3dc0f3. ± Comparison against base commit a760508.

Copy link
Contributor

@kon28289 kon28289 left a comment

Choose a reason for hiding this comment

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

패키지 구조 이해에 도움이 되었습니다!

Copy link
Contributor

@kimgunwooo kimgunwooo 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 +28 to +31
public class BoardWriter {
private final BoardJpaRepository boardJpaRepository;
private final CategoryJpaRepository categoryJpaRepository;
private final BoardCategoryJpaRepository boardCategoryJpaRepository;
Copy link
Contributor

Choose a reason for hiding this comment

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

P5 : 여기서 Jpa 라는 구현체를 나타낸 이유가 뭔가요?? BoardRepository 라는 하나의 인터페이스만 사용하고, 만약 countLikesByBoardId 라는 메서드에 추가 작업을 위해 Jpa -> QueryDSL 혹은 Jpa -> JDBC 로 바뀌더라도 상위 계층에 영향이 없는게 좋지 않나요??

Copy link
Member Author

Choose a reason for hiding this comment

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

앗 실수입니다 boardJpaRepository 가 아닌 boardRepository 만 있으면 됩니다

잘못 주입시킨거 같습니다

Comment on lines +19 to +21
public Optional<UserTarget> findUser(Long userId) {
return userRepository.findUser(userId);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

P5 : 해당 메서드를 사용하는 부분에서 결국 orElseThrow를 통한 예외처리를 동일하게 적어줘야 하는데, 해당 메서드 내에서 하지 않은 이유가 있나요??

Copy link
Member Author

Choose a reason for hiding this comment

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

예외처리 또한 비즈니스 로직에 책임이 더 강한 느낌이란 생각이 들어서 입니다.

물론 모든 로직에서 예외처리를 동일하게 던진다면 코드의 중복성 측면에서는 UserReader에 예외처리를 하는 것이 옳을 수도 있지만 어떤 로직은 예외가 아닌 단순 UserTarget만이 필요 할 수 있다고 판단해서 UserReader가 예외를 처리하는 것이 옳은가라는 생각을 해봤습니다

그리고 Service 쪽에서 예외를 처리하지 않는다면 UserService 의 비즈니스 코드의 userReader.findUser를 보고 User를 왜 찾는지 명확하지 않다고 생각이 들었습니다.
user를 검증하는 것을 비즈니스 코드에 보일려면 예외를 UserService에서 처리하는 것이 옳다고 판단했습니다

Copy link
Contributor

@patulus patulus left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 참고해서 리팩터링 진행하겠습니다!

Copy link
Contributor

@siwan9 siwan9 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 참고해서 리팩터링 진행하겠습니다

@Schema(description = "게시물 생성 요청")
public class BoardCreateRequest {
public record BoardCreateRequest (
Copy link
Contributor

Choose a reason for hiding this comment

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

P5 : 혹시 DTO에 record 키워드를 사용하는 것을 컨벤션으로 정하는게 좋다고 생각하시나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

record 사용하면 장점이 lombok 사용 안해도 보일러 코드를 사용 안해도 되고 불변 객체라는 장점이 있어서 사용하면 좋을거라 판단했습니다

근데 DTO를 record 로 컨벤션을 정하는건 취향따라 달리해도 된다고 생각합니다

@kang20 kang20 merged commit a50c42c into develop Feb 24, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
♻️ Refactor 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants